[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Plea to use STD{IN,OUT,ERR}_FILENO instead of constants [was: Re:
From:       Bernhard Fischer <rep.dot.nop () gmail ! com>
Date:       2007-10-29 21:10:25
Message-ID: 20071029211025.GE1869 () aon ! at
[Download RAW message or body]

On Mon, Oct 29, 2007 at 12:17:31PM -0700, vda@busybox.net wrote:
>Author: vda
>Date: 2007-10-29 12:17:29 -0700 (Mon, 29 Oct 2007)
>New Revision: 20322
>
>Log:
>ash: if tcgetattr(stdin) fails, don't mess with tcsetattr
>ash: size-optimize ulimit's table of limits
>
>   text    data     bss     dec     hex filename
> 777345     974    9676  787995   c061b busybox_old
> 777253     974    9676  787903   c05bf busybox_unstripped
>
>
>
>Modified:
>   trunk/busybox/shell/ash.c
>
>
>Changeset:
>Modified: trunk/busybox/shell/ash.c
>===================================================================
>--- trunk/busybox/shell/ash.c	2007-10-26 17:50:09 UTC (rev 20321)
>+++ trunk/busybox/shell/ash.c	2007-10-29 19:17:29 UTC (rev 20322)

> #if ENABLE_ASH_READ_TIMEOUT
> 	if (ts.tv_sec || ts.tv_usec) {
>-// TODO: replace with poll, it is smaller
> 		FD_ZERO(&set);
> 		FD_SET(0, &set);

It would be nice if (long-term) we could strive to use the respective
STD{IN,OUT,ERR}_FILENO defines for these.
> 
>-		i = select(FD_SETSIZE, &set, NULL, NULL, &ts);
>-		if (!i) {
>+		/* poll-based wait produces bigger code, using select */
>+		i = select(1, &set, NULL, NULL, &ts);

likewise.

PS: while for /me, it's perfectly clear what these usually boil down to,
it is more elaborate to read and easier to understand for beginners if
the defines are used. But more importantly, as Shaun Jackman and others
did point out more than once:
"
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.
"
So, IMHO, it's a matter of cleanlyness to use STD{IN,OUT,ERR}_FILENO
where possible.
PPS: I already did change several of the numerical FDs to their defined
names and am willing to donate time to do an extra path, once again, of
course..

cheers,
Bernhard
_______________________________________________
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]