[RFC] strrstr function in libbb failes on some corner cases

[Tito]

On Sunday 15 June 2008 23:21:43 Bernhard Fischer wrote:
> On Sun, Jun 15, 2008 at 11:06:17PM +0200, Tito wrote:
> >Hi,
> >while inspecting the last changes in busybox code I hit some issues in
> >libbb/strrstr.c. The strrtstr function fails on some corner cases of a little
> >test program I wrote (and bombs out on NULL pointers):
> 
> This was intended, see below.
> 
> >For the moment there is no diff but just a drop in replacement
> >for strrstr.c for testing and improvement by the list members.
> >There is a little size increase:
> 
> heh, i wouldn't call 50% a "little", but ok.
> 
> Ultimately i don't care, though. I just put in the smallest version for
> depmod knowing that it will segfault left and right if somebody throws
> NULL on it, since it was enough for depmod.
Hi,
To be honest I was tempted to remove the code that handles NULL pointers
in which case the size is about the same as before.

scripts/bloat-o-meter busybox_old busybox_unstripped
function                                             old     new   delta
strrstr                                               53      61      +8
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 1/0 up/down: 8/0)                 Total: 8 bytes

My point was that:
1) actually it fails to detect "aa" in "aaa"
2) this and other  limitations shown by my test program are not described 
    in the code nor in the function name so people in the future will think 
    they have a correct working implementation of strrstr
3) if it is good enough for depmod and used only by depmod then move it into depmod.c
    or fix it for future use.     
Ciao,
Tito


> Doesn't it suffice if we just
> if (haystack == NULL || needle == NULL) return NULL and keep the strcmp?
> 
> 
> >scripts/bloat-o-meter busybox_old busybox_unstripped
> >function                                             old     new   delta
> >strrstr                                               53      73     +20
> >------------------------------------------------------------------------------
> >(add/remove: 0/0 grow/shrink: 1/0 up/down: 20/0)               Total: 20 bytes
> >
> >The strrstr function at the moment is used only in depmod 
> >so the depmod author should check if this new implementation
> >is suitable to be used.
>