Re: [PATCH] add memrchr(3)

[Eric Blake <eblake at redhat dot com>]

On 05/09/2012 01:02 PM, Yaakov (Cygwin/X) wrote:
> On Wed, 2012-05-09 at 08:23 -0600, Eric Blake wrote: 
>> sizeof(unsigned char) is defined by C to be exactly 1; I always
>> question code that spells it out longhand instead of using 1.
> 
> But, but, magic numbers! :-) Not that I care either way here.
> 
>>>>       if (src > src_end) >> 	break;
>>
>> Also, src will never be > src_end - memchr returns NULL rather than
>> reading beyond the bounds of length.
> 
> Actually, it *was* returning values beyond src_end and the effects
> weren't pretty, hence the check.
> 
>> Additionally, I think that searching forwards through the array via one
>> function call per occurrence of the byte in question is wasteful -
>> since we already know the array bounds, we might as well search in
>> reverse by doing a single C loop that iterates backwards over a word at
>> a time. strrchr must search forwards, because it is also searching for
>> the terminating NUL and doesn't know the length in advance, but memrchr
>> should be faster.
> 
> Fair enough, so code duplication it is then.  Revised source file
> attached.

Looks good to me.

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature