[PATCH 1/7] bb_ioctl implementation - improved

Denis Vlasenko vda.linux at googlemail.com
Sun Jul 15 12:45:48 UTC 2007


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.

Well, manpages of most library functions say that on return -1 is returned
(when function returns an int/long/off_t). And in many places
we explicitly check for -1.

It can be a silly project for someone to provide FAILED_NEGATIVE(x) macro
which uses the most efficient error check for this case for given arch.
For i386 it will be (x) == -1, for many arches probably (x) < 0.
Then there are functions which return 0 on success and can be checked
with just (x)!=0 - like stat, close, dup. FAILED_NONZERO(x)?

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



More information about the busybox mailing list