[PATCH] Improve portability

Dan Fandrich dan at coneharvesters.com
Wed Sep 9 01:40:20 UTC 2009


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.

> > > 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.

> >  	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.

Attached is a reworked patch.

>>> Dan
-- 
http://www.MoveAnnouncer.com              The web change of address service
          Let webmasters know that your web site has moved
-------------- next part --------------
A non-text attachment was scrubbed...
Name: busybox-nogcc-final.diff
Type: text/x-patch
Size: 2215 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20090908/fdf20bb9/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 288 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20090908/fdf20bb9/attachment.pgp>


More information about the busybox mailing list