Subject: Re: RFC: memmem(3)
To: None <wulf@ping.net.au>
From: Robert Elz <kre@munnari.OZ.AU>
List: tech-userlevel
Date: 03/04/2003 17:05:09
    Date:        Sun, 2 Mar 2003 20:26:03 +1030 (CST)
    From:        wulf@ping.net.au
    Message-ID:  <200303020956.h229upW14620@gw.ping.net.au>

  | after many uses I like to propose memmem(3) for inclusion into libc.

Others have commented on the general issues, man page, func signature, ...

so, for me ...

  |         /* Sanity check */
  |         if(!(b1 && b2 && len1 && len2))
  |                 return NULL;

For style, I'd prefer
		if (b1 == NULL || b2 == NULL || len1 == 0 || len2 == 0)
			return NULL;

For correctness I'd prefer
		if (b1 == NULL || b2 == NULL || len2 <= 0 || len1 < len2)
			return NULL;

(the len1 < len2 just accomplishes the needed len1 <= 0, and also does
a quick exit in the case where a match cannot be found).

While it would be possible to give meaning to the cases where len1 or len2
was negative (the block of memory ending at b1 or b2 resp) the manual page
doesn't say that's supposed to work, and the rest of the code certainly
doesn't make it so.

  |         while (sp <= eos) {
  |                 if (*sp == *pp)
  |                         if (memcmp(sp, pp, len2) == 0)
  |                                 return sp;
  | 
  |                         sp++;
  |         }

There's no point comparing *sp to *pp twice, so you could...

	len2--;
	while (sp <= eos) {
		if (*sp++ == *pp && memcmp(sp, pp+1, len2) == 0)
			return sp-1;
	}

not that that kind of micro-optimisation probably makes any difference
worth having.

If this is done, and the case of len2==1 was likely (which it probably
isn't, as then we'd have memchr()) it could be tested (compared to 0)
before the memcmp to avoid the func call overhead.

kre