[BusyBox] Replace 0 with STDIN_FILENO [PATCH]

Shaun Jackman sjackman at gmail.com
Tue Mar 22 06:22:09 UTC 2005


On Mon, 21 Mar 2005 19:52:05 -0500, Rob Landley <rob at landley.net> wrote:
> > Symbolic constants are clearer.
> 
> Not really.  In this case, for me at least, it's work the other way...

Fair enough. I still prefer the name over the nameless number. For
instance, I typically refer to "pi", not 3.14159....

> Try going "make STDERR_FILENO>&STDOUT_FILENO | less" on the command line some
> time and see how far you get.  Anybody even remotely familiar with Unix knows
> that 0, 1, and 2 mean.

Shell and C code are similar in some areas, but it's a dangerous false
parallel to say that a common idiom in shell script is good C code.
 
> Partly I'm just startled that in 15 years of C programming I apparently
> haven't seen STDIN_FILENO enough times to remember it.  I suppose it must be
> a standard since you didn't #define it yourself.  Grepping through some other
> source code, it is occasionally used...  Exactly once in the linux-2.6.11
> sources, not at all in lilo-2.6.11, but twice in udev: once to #define it in
> klibc and once to use it...

$ grep -r FILENO busybox-1.00 | wc -l
91

> *shrug*.  So that promotes it to "pointless churn", I suppose.  Me, I have an
> aversion to wrappers that don't actually buy you anything.  I fail to see the
> point of factoring out a 30 year old single digit constant with a universally
> understood meaning...

If *_FILENO are always defined to *exactly* {0, 1, 2}, they would be
of questionable benefit. The single level of indirection does have a
benefit though, as below.

> > More importantly, it
> > allows a header to redefine STDIN_FILENO to fileno(stdin) so that
> > stdin may be replaced by a socket on a system that does not support
> > dup2.
> 
> If you close a filehandle the next one you open should be the lowest available
> filehandle, so in a nonthreaded environment (or with a thread-safe libc) you
> can simulate dup2 on the standard filehandles with close and open.  That IS
> standardized, and anything that doesn't support it is DEEPLY broken, and not
> Unix.

I'm not certain on this point, but I haven't run into a Posix spec so
far that dictates file descriptors *must* be allocated in order. As
far as I know, that's implementation dependent. In any case, it's
behaviour I wouldn't rely upon.

> Or did you introduce this arugment just to undermine your first argument, and
> show that assuming this macro is always going to be 0 is NOT a valid
> assumption that avoids the extra work of looking it up, because now it's
> hiding #defined infrastructure with a config option?

No, I didn't. I'm not wasting time on inconsequential rhetorical
arguments. I submitted this patch because I needed it for the reason I
gave above.

> (Admittedly, this is
> just new infrastructure that nobody's actually using and probably nobody ever
> will

I'm using it.

> and if they did try they'd have to audit every single file IO call in
> the system so that the ones that ARE using 0 don't break them.

No they wouldn't. vi was the *only* file I needed to patch in this
manner. I have 69 utilities in busybox enabled.

$ grep =y .config | wc -l
69

I would patch the other utilities as necessary.

> Is this now a
> requirement?  I'm going to continue to use 0.  If it's not a requirement, it
> doesn't gain you anything...)

Feel free to continue using {0, 1, 2}. You may submit code in any
format the maintainer will accept. If necessary, I will submit patches
to make that code portable.
 
> One of the reasons I like busybox isn't just that the binaries are small and
> efficient but the code is SIMPLE.  I like simple.  The GNU crap has #ifdefs
> in it for dead cray mainframes.

I appreciate simple code. I also appreciate portable code.

> Busybox has (until recently) never claimed
> to support anything but linux, with an option on other sane Posix
> architectures like BSD or MacOS X or something...

>From README...
   Full functionality requires Linux 2.2.x or better.  A large fraction of the
   code should run on just about anything.  While the current code is fairly
   Linux specific, it should be fairly easy to port the majority of the code
   to support, say, FreeBSD or Solaris, or Mac OS X, or even Windows (if you
   are into that sort of thing).

It may have been that in the past busybox didn't claim to be portable,
but it does now.

[clip]
> But even so, the straight use of contstant 0 wins:
>
> knoppix at ttyp1[busybox]$ find . -type f | xargs grep "STDIN_FILENO" | wc
>      38     182    2185
> knoppix at ttyp1[busybox]$ find . -type f | xargs grep "read(0" | wc
>      22     185    1425

Uh, no it didn't. 38 to 22, STDIN_FILENO won.
 
> Highly pointless churn, if you ask me.  (And if you submit a patch to change
> every occurrence of constant 0, 1, and 2 in the tree to _FILENO macros in the
> name of "consistency", I'll submit a patch the other way which is just as
> consistent, easy to read, and makes at least the source code smaller...)

Were it pointless churn, you may have a point. Since it has a clear
point, portability, the remaining argument is source code size, which
I suggest is a secondary goal to portability.
 
> P.S.  Don't mind me, it's my last week at Dell.  That place makes me grumpy.
> It's like I'm counting the last days until summer vacation...

Grumpiness aside, please consider first that the patches I submit
exist because I *use* them, not because I like keeping the maintainer
on his toes.

Regards,
Shaun



More information about the busybox mailing list