[PATCH] Improve portability

[Denys Vlasenko]

On Wednesday 09 September 2009 03:40, Dan Fandrich wrote:
> On Sun, Sep 06, 2009 at 12:47:39PM +0200, Denys Vlasenko wrote:
> > On Sunday 06 September 2009 07:01, Dan Fandrich wrote:
> > +#include <sys/types.h>
> >  #include <sys/utsname.h>               /* for uname(2) */
> >  
> > sys/types.h is included by libbb.h. Does sys/utsname.h require it?
> 
> Yes, for Watcom C anyway. Moving the #include after libbb.h should work.
> 
> > Here it still calls a function twice:
> > 
> > -               old_user = xstrdup(IF_FEATURE_UTMP(getlogin() ? : ) (pw = getpwuid(cur_uid)) ? pw->pw_name : "");
> > -               tty = xmalloc_ttyname(2) ? : "none";
> > +               old_user = xstrdup(IF_FEATURE_UTMP(getlogin() ? getlogin() : ) (pw = getpwuid(cur_uid)) ? pw->pw_name : "");
> > +               tty = xmalloc_ttyname(2);
> > +               if (!tty) {
> > +                       tty = "none";
> > +               }
> 
> I've reworked this to only call getlogin() once.

Apllied, thanks.


> > > > For readability, I changed some of these to use 
> > > the MAX() macro instead.
> > 
> > -#define  CALC_TOT_DIFF  ((unsigned)(p_jif->total - p_prev_jif->total) ? : 1)
> > +#define  CALC_TOT_DIFF  MAX((unsigned)(p_jif->total - p_prev_jif->total), 1)
> > 
> > I'm not sure this is better. Now we need to be sure we have MAX(),
> > and double exaluation may still happen, depending on gcc's smarts.
> 
> MAX is defined in libbb.h so it's always going to be there. gcc is also smart
> enough to only evaluate the expression once. These instances are called infrequently
> so it's not worth the reduction in readability trying to optimize it for other
> compilers that might not be as smart.

After compiling patched source:

# make bloat
function                                             old     new   delta
display_process_list                                1462    1465      +3
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 1/0 up/down: 3/0)                 Total: 3 bytes

Not applied. Can you provide a patch which does not increase code size?


> > >  	var *pos;
> > >  	struct nvblock_s *prev;
> > >  	struct nvblock_s *next;
> > > -	var nv[0];
> > > +	var nv[];
> > >  } nvblock;
> > >  
> > >  typedef struct tsplitter_s {
> > > 
> > > Explicitly zero-length arrays seems to be allowed in C++ but not C, which
> > > uses an empty array size instead.
> > 
> > Does it mean the same thing?
> 
> It sure looks like it. It's used in BusyBox to define a flexible array
> member, so doing it in the C99 approved way (without an explicit length)
> is better.

Applied.
--
vda