[PATCH 0/2] stat-macros: shared code from coreutils & findutils

[PATCH 0/2] stat-macros: shared code from coreutils & findutils

[James Youngman]

GNU findutils uses a section of code shared with coreutils' system.h
to implement the -ls action.  This particular section of code deals
with figuring out the number of blocks in a file, and the units of
st_nblocks.

Rather than continue to try to keep these bits of code in sync, I
propose to move the code into a gnulib module.  The stat-macros module
looked to me as if it's the right place, but since that module was
originally trivial, this change adds a couple of previously
nonexistent dependencies.

Apologies if I got anything badly wrong, this is essentially my first
gnulib module.  It's particularly likely for example that I documented
the module in the wrong place.  So, corrections gratefully received.

Jim, I hope you don't mind me listing you as a maintainer, I did so
simply because the code came from coreutils.

Thanks,
James.

[PATCH 1/2] stat-macros: Enhance to provide block-related information.

[James Youngman]

* lib/stat-macros.h: Insert part of coreutils' system.h, which
defines DEV_BSIZE, ST_BLKSIZE, ST_NBLOCKS and ST_NBLOCKSIZE.
* modules/stat-macros: Depend on sys_stat (because now
stat-macros.h includes sys/stat.h) and stdint (because we need
SIZE_MAX).  Since the file is now substantially composed of an
extract from coreutils system.h, which findutils shares, change
the maintainer to James Youngman, Jim Meyering.
* m4/stat-macros.m4: Check for sys/param.h.
* modules/stat-macros-tests: New test module for stat-macros.
* tests/test-stat-macros.c: New test, verifies very basic
behaviour of stat-macros.h.
---
 ChangeLog                 |   15 ++++++
 lib/stat-macros.h         |  116 ++++++++++++++++++++++++++++++++++++++++++++-
 m4/stat-macros.m4         |    3 +-
 modules/stat-macros       |    4 +-
 modules/stat-macros-tests |   14 +++++
 tests/test-stat-macros.c  |  115 ++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 264 insertions(+), 3 deletions(-)
 create mode 100644 modules/stat-macros-tests
 create mode 100644 tests/test-stat-macros.c

diff --git a/ChangeLog b/ChangeLog
index 62b99f3..c7ddc06 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,18 @@
+2011-06-06  James Youngman  <jay@gnu.org>
+
+	stat-macros: Enhance to provide block-related information.
+	* lib/stat-macros.h: Insert part of coreutils' system.h, which
+	defines DEV_BSIZE, ST_BLKSIZE, ST_NBLOCKS and ST_NBLOCKSIZE.
+	* modules/stat-macros: Depend on sys_stat (because now
+	stat-macros.h includes sys/stat.h) and stdint (because we need
+	SIZE_MAX).  Since the file is now substantially composed of an
+	extract from coreutils system.h, which findutils shares, change
+	the maintainer to James Youngman, Jim Meyering.
+	* m4/stat-macros.m4: Check for sys/param.h.
+	* modules/stat-macros-tests: New test module for stat-macros.
+	* tests/test-stat-macros.c: New test, verifies very basic
+	behaviour of stat-macros.h.
+
 2011-06-05  Bruno Haible  <bruno@clisp.org>
 
 	acl: Fix test failure on AIX 7.
diff --git a/lib/stat-macros.h b/lib/stat-macros.h
index 690216c..c196406 100644
--- a/lib/stat-macros.h
+++ b/lib/stat-macros.h
@@ -1,3 +1,117 @@
+/* macros useful in interpreting values in struct stat.
+   Copyright (C) 1989, 1991-2011 Free Software Foundation, Inc.
+
+   This program is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation, either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+/*
+   Macros defined by this file (s is an rvalue of type struct stat):
+   CHMOD_MODE_BITS: All the mode bits that can be affected by chmod.
+   DEV_BSIZE:       The device blocksize.  But use ST_NBLOCKSIZE instead.
+   ST_BLKSIZE(s):   Preferred (in the sense of best performance) I/O blocksize
+                    for the file, in bytes.
+   ST_NBLOCKS(s):   Number of blocks in the file, including indirect blocks.
+   ST_NBLOCKSIZE:   Size of blocks used when calculating ST_NBLOCKS.
+ */
+#ifndef STAT_MACROS_H
+# define STAT_MACROS_H
+
+# include <stdint.h>		/* We need to know SIZE_MAX */
+# include <sys/stat.h>
+# if HAVE_SYS_PARAM_H
+#  include <sys/param.h>
+# endif
+
+
+
 /* All the mode bits that can be affected by chmod.  */
-#define CHMOD_MODE_BITS \
+# define CHMOD_MODE_BITS \
   (S_ISUID | S_ISGID | S_ISVTX | S_IRWXU | S_IRWXG | S_IRWXO)
+
+
+/* Much of the remainder of this file is not indented consistently
+   with the above, in order to make it easier to see that the text
+   is almost identical to part of the system.h header in coreutils.
+*/
+/* Get or fake the disk device blocksize.
+   Usually defined by sys/param.h (if at all).  */
+#if !defined DEV_BSIZE && defined BSIZE
+# define DEV_BSIZE BSIZE
+#endif
+#if !defined DEV_BSIZE && defined BBSIZE /* SGI sys/param.h */
+# define DEV_BSIZE BBSIZE
+#endif
+#ifndef DEV_BSIZE
+# define DEV_BSIZE 4096
+#endif
+
+/* Extract or fake data from a `struct stat'.
+   ST_BLKSIZE: Preferred I/O blocksize for the file, in bytes.
+   ST_NBLOCKS: Number of blocks in the file, including indirect blocks.
+   ST_NBLOCKSIZE: Size of blocks used when calculating ST_NBLOCKS.  */
+#ifndef HAVE_STRUCT_STAT_ST_BLOCKS
+# define ST_BLKSIZE(statbuf) DEV_BSIZE
+/* coreutils' fileblocks.c also uses BSIZE.  */
+# if defined _POSIX_SOURCE || !defined BSIZE 
+#  define ST_NBLOCKS(statbuf) \
+  ((statbuf).st_size / ST_NBLOCKSIZE + ((statbuf).st_size % ST_NBLOCKSIZE != 0))
+# else /* !_POSIX_SOURCE && BSIZE */
+#  define ST_NBLOCKS(statbuf) \
+  (S_ISREG ((statbuf).st_mode) \
+   || S_ISDIR ((statbuf).st_mode) \
+   ? st_blocks ((statbuf).st_size) : 0)
+# endif /* !_POSIX_SOURCE && BSIZE */
+#else /* HAVE_STRUCT_STAT_ST_BLOCKS */
+/* Some systems, like Sequents, return st_blksize of 0 on pipes.
+   Also, when running `rsh hpux11-system cat any-file', cat would
+   determine that the output stream had an st_blksize of 2147421096.
+   Conversely st_blksize can be 2 GiB (or maybe even larger) with XFS
+   on 64-bit hosts.  Somewhat arbitrarily, limit the `optimal' block
+   size to SIZE_MAX / 8 + 1.  (Dividing SIZE_MAX by only 4 wouldn't
+   suffice, since "cat" sometimes multiplies the result by 4.)  If
+   anyone knows of a system for which this limit is too small, please
+   report it as a bug in this code.  */
+# define ST_BLKSIZE(statbuf) ((0 < (statbuf).st_blksize \
+                               && (statbuf).st_blksize <= SIZE_MAX / 8 + 1) \
+                              ? (statbuf).st_blksize : DEV_BSIZE)
+# if defined hpux || defined __hpux__ || defined __hpux
+/* HP-UX counts st_blocks in 1024-byte units.
+   This loses when mixing HP-UX and BSD file systems with NFS.  */
+#  define ST_NBLOCKSIZE 1024
+# else /* !hpux */
+#  if defined _AIX && defined _I386
+/* AIX PS/2 counts st_blocks in 4K units.  */
+#   define ST_NBLOCKSIZE (4 * 1024)
+#  else /* not AIX PS/2 */
+#   if defined _CRAY
+#    define ST_NBLOCKS(statbuf) \
+  (S_ISREG ((statbuf).st_mode) \
+   || S_ISDIR ((statbuf).st_mode) \
+   ? (statbuf).st_blocks * ST_BLKSIZE (statbuf) / ST_NBLOCKSIZE : 0)
+#   endif /* _CRAY */
+#  endif /* not AIX PS/2 */
+# endif /* !hpux */
+#endif /* HAVE_STRUCT_STAT_ST_BLOCKS */
+
+#ifndef ST_NBLOCKS
+# define ST_NBLOCKS(statbuf) ((statbuf).st_blocks)
+#endif
+
+#ifndef ST_NBLOCKSIZE
+# ifdef S_BLKSIZE
+#  define ST_NBLOCKSIZE S_BLKSIZE
+# else
+#  define ST_NBLOCKSIZE 512
+# endif
+#endif
+
+#endif /* STAT_MACROS_H */
diff --git a/m4/stat-macros.m4 b/m4/stat-macros.m4
index cd86c09..c575f68 100644
--- a/m4/stat-macros.m4
+++ b/m4/stat-macros.m4
@@ -1,4 +1,4 @@
-#serial 3
+#serial 4
 
 # Copyright (C) 2005-2006, 2009-2011 Free Software Foundation, Inc.
 #
@@ -9,4 +9,5 @@
 AC_DEFUN([gl_STAT_MACROS],
 [
   AC_REQUIRE([AC_HEADER_STAT])
+  AC_CHECK_HEADERS([sys/param.h])
 ])
diff --git a/modules/stat-macros b/modules/stat-macros
index d9a331c..68fa6dc 100644
--- a/modules/stat-macros
+++ b/modules/stat-macros
@@ -5,6 +5,8 @@ Files:
 lib/stat-macros.h
 
 Depends-on:
+sys_stat
+stdint
 
 configure.ac:
 
@@ -17,4 +19,4 @@ License:
 GPL
 
 Maintainer:
-all
+James Youngman, Jim Meyering
diff --git a/modules/stat-macros-tests b/modules/stat-macros-tests
new file mode 100644
index 0000000..d56481c
--- /dev/null
+++ b/modules/stat-macros-tests
@@ -0,0 +1,14 @@
+Files:
+tests/test-stat-macros.c
+
+Depends-on:
+fcntl
+open
+stat
+unistd
+
+configure.ac:
+
+Makefile.am:
+TESTS += test-stat-macros
+check_PROGRAMS += test-stat-macros
diff --git a/tests/test-stat-macros.c b/tests/test-stat-macros.c
new file mode 100644
index 0000000..b312e14
--- /dev/null
+++ b/tests/test-stat-macros.c
@@ -0,0 +1,115 @@
+/* Tests of stat-macros.h.
+   Copyright (C) 2011 Free Software Foundation, Inc.
+
+   This program is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <config.h>
+#include "stat-macros.h"
+
+#include <stdio.h>		/* perror */
+#include <fcntl.h>		/* open */
+#include <unistd.h>		/* unlink, close */
+
+/*
+ST_NBLOCKS may be 0
+
+: test ST_NBLOCKSIZE for nonzeroness
+: ST_NBLOCKS(s) by creating a file.
+: ST_BLKSIZE(s) by creating a file.
+*/
+
+#include "macros.h"
+
+
+/* Create a test file containing more than one block, 
+   and return nonzero if successful. */
+static int
+create_test_file (const char *name)
+{
+  int retval = 0;
+  char buf[ST_NBLOCKSIZE];	/* contents are irrelevant */
+  int fd = open (name, O_CREAT|O_WRONLY, 0700);
+  if (fd >= 0)
+    {
+      if (write (fd, buf, ST_NBLOCKSIZE) >= 0)
+	{
+	  if (write (fd, buf, ST_NBLOCKSIZE) >= 0)
+	    retval = 1;
+	  else
+	    perror (name);
+	}
+      else
+	{
+	  perror (name);
+	}
+      close (fd);
+    }
+  else
+    {
+      perror (name);
+    }
+  return retval;
+}
+
+
+static void
+test_nblocksize (void)
+{
+  ASSERT (ST_NBLOCKSIZE > 0);
+}
+
+static void
+test_nblocks (const struct stat *p)
+{
+  ASSERT (ST_NBLOCKS (*p) > 0);
+}
+
+
+static void
+test_blksize (const struct stat *p)
+{
+  ASSERT (ST_BLKSIZE (*p) > 0);
+}
+
+
+int main (int argc, char *argv[])
+{
+  const char *test_file_name = "test-statmacros.txt";
+  int result = 0;
+
+  test_nblocksize ();
+  if (create_test_file (test_file_name))
+    {
+      struct stat st;
+      if (0 == stat (test_file_name, &st))
+	{
+	  ASSERT (st.st_size > 1);
+	  test_nblocks (&st);
+	  test_blksize (&st);
+	}
+      else
+	{
+	  perror (test_file_name);
+	  result = 1;
+	}
+    }
+  else
+    {
+      result = 1;
+    }
+  /* Even if create_test_file failed for some reason, the test
+     file may still exist. */
+  (void) unlink (test_file_name);
+  return result;
+}
-- 
1.7.2.5

[PATCH 2/2] stat-macros: document the stat-macros module.

[James Youngman]

* doc/gnulib.texi: Include stat-macros.texi.
* doc/stat-macros.texi: Documentation for this module.
---
 ChangeLog            |    6 ++++++
 doc/gnulib.texi      |    4 ++++
 doc/stat-macros.texi |   31 +++++++++++++++++++++++++++++++
 3 files changed, 41 insertions(+), 0 deletions(-)
 create mode 100644 doc/stat-macros.texi

diff --git a/ChangeLog b/ChangeLog
index c7ddc06..3f74eea 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2011-06-07  James Youngman  <jay@gnu.org>
+
+	stat-macros: document the stat-macros module.
+	* doc/gnulib.texi: Include stat-macros.texi.
+	* doc/stat-macros.texi: Documentation for this module.
+
 2011-06-06  James Youngman  <jay@gnu.org>
 
 	stat-macros: Enhance to provide block-related information.
diff --git a/doc/gnulib.texi b/doc/gnulib.texi
index 1e1ad40..416d48c 100644
--- a/doc/gnulib.texi
+++ b/doc/gnulib.texi
@@ -6512,6 +6512,7 @@ This list of functions is sorted according to the header that declares them.
 * warnings::
 * manywarnings::
 * Running self-tests under valgrind::
+* stat-macros::
 @end menu
 
 @node alloca
@@ -6608,6 +6609,9 @@ ASCII characters.
 
 @include valgrind-tests.texi
 
+@include stat-macros.texi
+
+
 @node Regular expressions
 @chapter Regular expressions
 
diff --git a/doc/stat-macros.texi b/doc/stat-macros.texi
new file mode 100644
index 0000000..b925032
--- /dev/null
+++ b/doc/stat-macros.texi
@@ -0,0 +1,31 @@
+@node stat-macros
+@section stat-macros
+
+The @code{stat-macros} module provides a small number of macros
+intended for interpreting the information in an instance of
+@code{struct stat}.
+
+@c not a function, but index it anyhow.
+@findex CHMOD_MODE_BITS
+This macro is a bitmask indicating which bits in the @code{st_mode}
+member of @code{struct stat} can be affected by @code{chmod}.
+
+@c We deliberately don't document DEV_BSIZE (it looks to James
+@c Youngman as if the ST_NBLOCKSIZE macro should be used instead).
+
+@findex ST_NBLOCKS
+@findex ST_NBLOCKSIZE
+@cindex block size
+On POSIX systems, the @code{st_blocks} member of @code{struct stat}
+contains the number of disk blocks occupied by a file.  The
+@code{ST_NBLOCKS} macro is used to estimate this quantity on systems
+which don't actually have @code{st_blocks}.  Each of these blocks
+contains @code{ST_NBLOCKSIZE} bytes.
+
+@findex ST_BLKSIZE
+The value of @code{ST_NBLOCKSIZE} is often quite small, small enough
+that performing I/O in chunks that size would be inefficient.
+@code{ST_BLKSIZE} is the I/O block size recommended for I/O to this
+file.  This is not guaranteed to give optimum performance, but it
+should be reasonably efficient.
+
-- 
1.7.2.5

Re: [PATCH 1/2] stat-macros: Enhance to provide block-related information.

[Paul Eggert]

It'd be good to add something like this, thanks.

A few comments:

The time-related stat macros are already broken out into
stat-time.h.   Perhaps these size-related macros should
be broken out into stat-size.h?  That way, we could
leave stat-macros.h alone.  Many programs that need
CHMOD_MODE_BITS (in the current stat-macros.h) don't need
the heavy artillery used to configure the size-related macros.
If we make this change, the module should be renamed to
"stat-size" of course.

The indenting for the '#' lines in stat-macros.h isn't
consistent.  The usual gnulib style is to not count
"#ifndef STAT_MACROS_H" as a separate indenting level.

> +#  define ST_NBLOCKS(statbuf) \
> +  (S_ISREG ((statbuf).st_mode) \
> +   || S_ISDIR ((statbuf).st_mode) \
> +   ? st_blocks ((statbuf).st_size) : 0)

This indenting doesn't look right, as || and ? should be
different levels.  I'd combine the 2nd and 3rd lines into one line.
There's another definition of ST_NBLOCKS that has the same problem.
(I realize this problem is in coreutils but we might as well fix it....)

> +#   endif /* _CRAY */
> +#  endif /* not AIX PS/2 */
> +# endif /* !hpux */
> +#endif /* HAVE_STRUCT_STAT_ST_BLOCKS */

I'd omit comments on the "else" and "endif" lines, such as
these, as they make the code harder to read.  They can be
helpful for long #if blocks, but these are short.

> +  AC_CHECK_HEADERS([sys/param.h])

This should be AC_CHECK_HEADERS_ONCE.
 
>  Depends-on:
> +sys_stat
> +stdint

We can break the dependency on the stdint module
by using (size_t) -1 instead of SIZE_MAX.

Why does stat-macros need to depend on sys_stat?
Unless there's a specific need for the dependency,
I'd remove it.

Re: [PATCH 1/2] stat-macros: Enhance to provide block-related information.

[Jim Meyering]

Paul Eggert wrote:
> It'd be good to add something like this, thanks.

Thanks, James.

I like your idea, too.
Those definitions are duplicated in far too many packages.

> A few comments:
>
> The time-related stat macros are already broken out into
> stat-time.h.   Perhaps these size-related macros should
> be broken out into stat-size.h?  That way, we could
> leave stat-macros.h alone.  Many programs that need
> CHMOD_MODE_BITS (in the current stat-macros.h) don't need
> the heavy artillery used to configure the size-related macros.
> If we make this change, the module should be renamed to
> "stat-size" of course.

In the spirit of dependency minimization, it does sound like
a good idea to put this in a new module.

Re: [PATCH 1/2] stat-macros: Enhance to provide block-related information.

[James Youngman]

On Wed, Jun 8, 2011 at 1:43 AM, Paul Eggert <eggert@cs.ucla.edu> wrote:
>>  Depends-on:
>> +sys_stat
>> +stdint
>
> We can break the dependency on the stdint module
> by using (size_t) -1 instead of SIZE_MAX.

Thanks for the tip.

> Why does stat-macros need to depend on sys_stat?

Because ST_NBLOCKS uses S_ISREG, for example.

> Unless there's a specific need for the dependency,
> I'd remove it.

James.