glitch with strlen [was: Re: Shells in 1.1.2]

[Rob Landley]

On Friday 05 May 2006 3:40 am, Bernhard Fischer wrote:
> >We only need bb_strlen() on gcc anyway, since other compilers may not
> > force inline strlen() in the first place.  But we're not compiler
> > agnostic, and last I checked we weren't even pretending to be. 
> > (Platform, yes.  Compiler, no.)
>
> I don't know, but i try to write C code for busybox. YMMV :P

Sure.  But c99 is c code, calls to libc are c code, all that #ifdef 
__uClinux__ stuff is C code...

> >Sigh.  bb_strlen() always struck me as a marginal optimization.  The
> >complexity is _almost_ worth it, but it keeps breaking.  I remember
> >complaining when the #undef plus #include "libbb.h" in the middle of the
> > file went in.  UGLY...
> >
> >Alright, I just checked in a fix.  It's not a non-ugly fix, but it's at
> > least less ugly than what was there.
>
> This is not a fix but breaks the compilation for non-gcc compiles.

A) You'll notice that the #define strlen(x) bb_strlen(x) is now in platform.h, 
and takes place within an #ifdef __GNUC__.  I didn't put an #ifdef around the 
actual definition of the function in libbb.c because that properly belongs in 
the makefile, but only gcc will ever _use_ it and fixing the build on other 
compilers is just a question of getting the dependencies right.

B) Show me one non-gcc compiler that built the tree before this checkin.  (I'm 
honestly interested: are there any?  TCC and Intel's C compiler emulate gcc 
in order to build the linux kernel, they might manage it, but I don't know 
anybody who's tried it.)  Show me something it breaks that wasn't broken 
before.

> Can we _discuss_ how to fix this properly?

Sure.

> This optimization helps at least with gcc which is what most people will
> use, probably.
> I suggested above that we don't rely on the preprocessor but fix the
> applets proper to use bb_strlen. This would IMO be a clean solution 
> which is safe and trivial.
>
> Thoughts?

I'd honestly rather rip the optimization out entirely than sprinkle 
bb_strlen() around the tree to work around a gcc "optimization" we can't 
switch off.  There's an -fno-builtin-strlen, but it doesn't work.  You're 
proposing renaming 530 occurrences of strlen to work around a gcc bug.  That 
does not strike me as a good idea.

Rob
-- 
Never bet against the cheap plastic solution.