Re: [PATCH] Add git-imap-send.

From: Linus Torvalds <torvalds@osdl.org>
Date: Mar 9, 2006, 16:41 UTC
Message-ID: <Pine.LNX.4.64.0603090836030.18022@g5.osdl.org>
In-Reply-To: <Pine.LNX.4.63.0603091227560.20277@wbgn013.biozentrum.uni-wuerzburg.de>

On Thu, 9 Mar 2006, Johannes Schindelin wrote:

Show 9 quoted lines

> > 
> > > +		if (!memcmp( "imaps:", val, 6 )) {
> > > +			if (!memcmp( "imap:", val, 5 ))
> > 
> > Is val always longer than 5 or 6 bytes here?
> 
> That does not matter, since they are strings, and the memcmp should not 
> look further if they are shorter (because the comparison to '\0' failed 
> already).

No.

It's true that any sane memcmp() will stop when it notices a difference, and it's also true that the return value semantics of memcmp() means that it has to walk beginning-to-end.

HOWEVER. The key phrase is "_when_ it notices a difference".

It's quite common for optimized memcmp()'s to do things like loading several words from both the source and the destinations, and testing them together, and only start doing the byte-by-byte comparison when the "big" comparison has failed.

So when you do a

	if (!memcmp(string, mystring, mystringlength))
		...

it's entirely possible that it will load bytes from "string" _past_ the end of the string because of an unrolled inner loop that does things multiple bytes at a time. They won't be used in the eventual result, but just the fact that they are loaded from memory can mean that your program takes a SIGSEGV, for example, becaue it turns out "string" was just a single NUL byte at the end of a page, and there's nothing after it.

IOW, it's a bad optimization.

Use "strncmp()" instead. Yes, it can be slower, exactly because it has to check more, but it checks more exactly because memcmp() can cause undefined behaviour by running off the end of a string.

		Linus