GNU bug report logs - #9077
coreutils-8.12: fiemap.h uses "struct fiemap_extent fm_extents[0];"

Previous Next

Full log

Message #21 received at 9077-done <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Pádraig Brady <P <at> draigBrady.com>
Cc: 9077-done <at> debbugs.gnu.org, Joachim Schmitz <jojo <at> schmitz-digital.de>
Subject: Re: bug#9077: coreutils-8.12: fiemap.h uses "struct fiemap_extent
	fm_extents[0]; "
Date: Fri, 15 Jul 2011 15:45:31 -0700

On 07/15/11 02:49, Pádraig Brady wrote:
> +#ifdef __linux__
> +  struct fiemap_extent fm_extents[0];
> +#endif

This assumes that all GNU/Linux compiles will
use a compiler that allows size-zero arrays;
also, it assumes that sizeof (struct fiemap)
== offsetof (struct fiemap, fm_extents).
Although both assumptions are probably
portable in practice in this particular case,
it's better to avoid them, if avoiding them is easy.
This will help future-proof the code if we alter the
data structures, as altering them might cause problems
such as what I recently encountered in GNU Emacs's C code.

How about the following further patch?  It should do the trick.

build: more-portable fix for fiemap compile failure
* src/extent-scan.c: Include <stddef.h>, for offsetof.
(extent_scan_read): Don't assume that sizeof (struct fiemap) is
equal to offsetof (struct fiemap, fm_extents).  Although this
should be a portable assumption in practice for this particular
case, the C standard doesn't guarantee it, and we might as well do
it the right way.  As it happens I recently got burned by this
exact issue in the source code for GNU Emacs; see
<http://debbugs.gnu.org/cgi/bugreport.cgi?bug=8884>.
* src/fiemap.h (struct fiemap.fm_extents): Change size back to 1.
diff --git a/src/extent-scan.c b/src/extent-scan.c
index 37445b8..434258f 100644
--- a/src/extent-scan.c
+++ b/src/extent-scan.c
@@ -17,6 +17,7 @@
    Written by Jie Liu (jeff.liu <at> oracle.com).  */

 #include <config.h>
+#include <stddef.h>
 #include <sys/types.h>
 #include <sys/ioctl.h>
 #include <sys/utsname.h>
@@ -96,7 +97,10 @@ extent_scan_read (struct extent_scan *scan)
       union { struct fiemap f; char c[4096]; } fiemap_buf;
       struct fiemap *fiemap = &fiemap_buf.f;
       struct fiemap_extent *fm_extents = &fiemap->fm_extents[0];
-      enum { count = (sizeof fiemap_buf - sizeof *fiemap)/sizeof *fm_extents };
+      enum {
+        size = sizeof fiemap_buf - offsetof (struct fiemap, fm_extents),
+        count = size / sizeof *fm_extents
+      };
       verify (count > 1);

       /* This is required at least to initialize fiemap->fm_start,
diff --git a/src/fiemap.h b/src/fiemap.h
index 15ddff9..60561d3 100644
--- a/src/fiemap.h
+++ b/src/fiemap.h
@@ -52,12 +52,9 @@ struct fiemap
   uint32_t fm_reserved;

   /* Array of mapped extents(out).
-     This is protected by the ifdef because it uses non standard
-     zero length arrays.  Note C99 has the equivalent flexible arrays,
-     but we don't use those for maximum portability to older systems.  */
-# ifdef __linux__
-  struct fiemap_extent fm_extents[0];
-# endif
+     This uses a size 1 array to be compatible with compilers that
+     don't support C99's flexible arrays or GCC's size-zero arrays.  */
+  struct fiemap_extent fm_extents[1];
 };

 /* The maximum offset can be mapped for a file.  */

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.