[PATCH] Improve portability

[Denys Vlasenko]

On Sunday 06 September 2009 07:01, Dan Fandrich wrote:
> On Sun, Sep 06, 2009 at 02:23:58AM +0200, Denys Vlasenko wrote:
> > Applied part of the patch (see attached). If you want
> > the rest to be applied, please explain why each kind
> > of change is necessary or useful.
> 
> The remaining patches fall mostly into these four categories:
> 
> 1) Empty struct initializers, e.g.
> 
>  static const struct suffix_mult sfx[] = {
>     { "s", 1 },
>     { }	 		// <= should be { "", 0 }
>  };
> 
> I'm unable to determine if this is actually allowed by ANSI C or not, but
> at least two compilers I've tried don't handle it.

Ok

> 2) Automatic variables used as initializers, e.g.
> 
>  char delim = 1;
>  const char delimiter[2] = { delim, 0 };
> 
> C90 requires automatic struct variables to have initializers containing
> only constant expressions.



> 3) #include files added to satisfy dependencies required by system include
> files that are not self-contained, which happens in some non-glibc systems.
> Requiring <sys/socket.h> before <netinet/in.h> seems to be a relatively 
> common one on pre-SuS systems.

+#include <sys/types.h>
 #include <sys/utsname.h>               /* for uname(2) */
 
 #include "libbb.h"


-#include <sys/param.h>  /* MAXHOSTNAMELEN */
+#include <sys/types.h>
 #include <sys/utsname.h>
 #include "libbb.h"


sys/types.h is included by libbb.h. Does sys/utsname.h require it?


> 
> 4) Conditional operator missing the second expression, e.g.
> 
>   getlogin() ? : getpwuid(cur_uid))
> 
> Most of these were taken up from the first patch, but the ones that weren't
> seem to be the ones that required a function call be called again in the
> second expression. In those cases, I added a temporary variable to hold the
> result of the function to avoid a duplicate expensive function call and/or
> potential race condition.

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";
+               }


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


> The remaining two patches are these:
> 
> diff --git a/editors/awk.c b/editors/awk.c
> index cef7334..4c2837a 100644
> --- a/editors/awk.c
> +++ b/editors/awk.c
> @@ -114,7 +114,7 @@ typedef struct nvblock_s {
>  	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?


I am applying the attached patch. If you want to add more,
please send a new patch, rediffed against current git.

--
vda


-------------- next part --------------
A non-text attachment was scrubbed...
Name: 1.patch
Type: text/x-diff
Size: 11485 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20090906/80047d2b/attachment.bin>