[PATCH V2 2/8] string: fix strncmp function

Russell King - ARM Linux linux at arm.linux.org.uk
Wed Dec 18 07:52:03 EST 2013


On Wed, Dec 18, 2013 at 01:23:26PM +0100, Daniel Mack wrote:
> On 12/18/2013 01:15 PM, Russell King - ARM Linux wrote:
> > For instance, can you be sure that there aren't any uses of strncmp()
> > which already test for less-than-zero etc?
> 
> In this case, yes. The code as it stands is very small, and users only
> check for the return value of these functions to be 0.
> 
> But I wonder whether that patch is needed at all. At least the rest of
> this series does not even seem to introduce new call sites of strcmp()
> or strncmp() ...

The implementation appears to be buggy as it stands - all it does is
sum up the differences in the whole string.

What if you're comparing two strings:

	ac
	ca

The first character has a difference of +2, the second has a difference
of -2.  Sum those together and you get zero.  Oh, the two strings are
identical!

Oh, and the other nitpick with the code is this:

	*(a++)

Really, are those parens really needed?  If you don't know the precendence
there, then you really shouldn't be programming in C, because this might
also be buggy:

	*(a++) - *(b++)

because if we declare that we don't know the precedence rules, it could be
that this is evaluated as *((a++) - (*(b++))) which would lead to errors!
Maybe some more parens should be added to make it clear!  Or maybe we
should just learn the precedence rules and realise that:

	*a++ - *b++

is correct and clear and there's no need for any stupid idiotic parens
here.

Yes, I loath unnecessary parens.



More information about the linux-arm-kernel mailing list