[PATCH 1/7] bb_ioctl implementation - improved

Robin Farine robin.farine at terminus.org
Mon Jul 16 07:59:31 UTC 2007


On Sun July 15 2007 14:45, Denis Vlasenko wrote:
> On Sunday 15 July 2007 11:49, Bernhard Fischer wrote:
> > On Sat, Jul 14, 2007 at 08:42:34PM +0100, Denis Vlasenko wrote:
> > >On Wednesday 11 July 2007 23:13, Bernhard Fischer wrote:
> > >> >I know but i'm pretty sure that in all places in busybox
> > >> > where this test was used [ !=0 ] it errored out, i double
> > >> > checked it, but nonetheless maybe its a good idea to
> > >> > change it to [ < 0 ] to be more coherent with the original
> > >> > ioctl call. So attached is a fixed patch.
> > >>
> > >> checking for != 0 is imho not a good idea as opposed to <0.
> > >> The idea is sound in my POV, i didn't have a chance to
> > >> review it, though
> > >
> > >This was quite puzzling for me. Try to guess which is smallest
> > > (on i386)? * if (f() != 0)
> > >* if (f() < 0)
> > >* if (f() == -1)
> > >
> > >Third one is smallest! gcc just does "inc eax; j[n]z label";
> > > which is just 3 bytes:
> >
> > The third one is relying on implementation (un-)defined
> > behaviour.

> It is on that fuzzy bounbary "do we want to go that far
> for saving 1 byte of code?".

Especially when on ARM for instance, the == -1 variant is 8 bytes 
bigger than the < 0 for a given compiler and flags. The outcome of 
this kind of nano-optimization is compiler specific, it may change 
from version to version, and it depends on the target architecture 
and compilation flags. In my opinion, it makes much more sense to 
go for the safe variant rather than the one that spares 1 byte on 
architecture foo.

Robin



More information about the busybox mailing list