Re: Ad PR libc/1730: glibc bug in memmem()

[Mark Kettenis <kettenis at wins dot uva dot nl>]

   Date: Wed, 17 May 2000 10:06:04 -0400
   From: Greg Hudson <ghudson@MIT.EDU>

   The code part of the patch looks fine.  (I would probably avoid even
   computing the invalid pointer value in the haystack_len < needle_len,
   but I hardly think that's important.)

   > 	* sysdeps/generic/memmem.c (memmem): Check for invalid parameter.

   I don't think having haystack_len < needle_len constitutes an invalid
   parameter.

You must be a mathematician :-).  Looking for a needle that's bigger
than the haystack is definitely unphysical.

Anyway, I'm not sure there is a bug here.  The memmem() function is a
GNU extension.  We might very well say that haystack_len < needle_len
invokes undefined behaviour.  In fact since the memmem() in glibc is
*the* GNU implementation, it's reasonable to say that the
implementation defines its behaviour and that haystack_len <
needle_len is indeed undefined.

On the other hand, the overhead of the extra check is negligible on a
typical memmem() operation.  So it might be reasonable to have this
improvement.

   > +  /* Sanity check, otherwise the loop will search through the whole
   > +     memory.  */

   This comment might be confusing to some readers.  On most C
   implementations, most of the time, the loop will not search through
   the whole memory.  The particular case that bit me was that haystack
   was NULL (the result of malloc(0)), so the invalid computed pointer
   wrapped around to a very high value.

   Replacing "will" with "might" would probably fix the situation.

   Sorry to nitpick so much.  Thanks for your time.

Since having good documentation is in a sense more important than a
correct implementation, this kind of nitpicking is certainly welcome
:-).

Mark