Re: should mkdir depend on xalloc?

[Jim Meyering]

John W. Eaton wrote:
> Although lib/mkdir.c includes xalloc.h, the mkdir module doesn't
> depend on xalloc.  Should it?  If so, how about the following change?

Thanks for spotting that!
As you saw from Eric's reply, the inclusion of alloca.h was unnecessary.

This highlights a slightly larger problem.
While coreutils and other packages automatically detect this precise
buglet by running "make syntax-check" (which uses gnulib's own maint.mk),
that rule looks only at files from its version control repository.

Manually aiming that rule at gnulib spotted a few culprits
and showed there was room for improvement in the syntax-check rule.
We need a way to make it easy to apply such rules to all of gnulib...


>From 28e6fc0a9042637a27233dca31962afd53bbb8cd Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Tue, 5 Jan 2010 15:10:02 +0100
Subject: [PATCH 1/3] xalloc.h: use consistent formatting

* lib/xalloc.h: Move declarations to start in the first column.
---
 ChangeLog    |    5 +++++
 lib/xalloc.h |    8 ++++----
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 2e24e7f..55c4772 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2010-01-05  Jim Meyering  <address@hidden>
+
+       xalloc.h: use consistent formatting
+       * lib/xalloc.h: Move declarations to start in the first column.
+
 2010-01-05  Eric Blake  <address@hidden>

        mkdir: avoid xalloc
diff --git a/lib/xalloc.h b/lib/xalloc.h
index a565301..6122cc5 100644
--- a/lib/xalloc.h
+++ b/lib/xalloc.h
@@ -106,10 +106,10 @@ char *xstrdup (char const *str) ATTRIBUTE_MALLOC;
 # if HAVE_INLINE
 #  define static_inline static inline
 # else
-   void *xnmalloc (size_t n, size_t s) ATTRIBUTE_MALLOC;
-   void *xnrealloc (void *p, size_t n, size_t s);
-   void *x2nrealloc (void *p, size_t *pn, size_t s);
-   char *xcharalloc (size_t n) ATTRIBUTE_MALLOC;
+void *xnmalloc (size_t n, size_t s) ATTRIBUTE_MALLOC;
+void *xnrealloc (void *p, size_t n, size_t s);
+void *x2nrealloc (void *p, size_t *pn, size_t s);
+char *xcharalloc (size_t n) ATTRIBUTE_MALLOC;
 # endif

 # ifdef static_inline
--
1.6.6.384.g14e6a


>From 0c2fded89d2ebb3395d176001182ef710d68d430 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Tue, 5 Jan 2010 15:12:15 +0100
Subject: [PATCH 2/3] maint.mk: include 4 more function names in 
alloca.h-checking regexp

* top/maint.mk (sc_prohibit_xalloc_without_use): Use more complete
regexp.  Before, we would give a false-positive (saying alloca.h
is included unnecessarily) when the only uses involved omitted symbols.
---
 ChangeLog    |    5 +++++
 top/maint.mk |   12 +++++++++---
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 55c4772..1313708 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
 2010-01-05  Jim Meyering  <address@hidden>

+       maint.mk: include 4 more function names in alloca.h-checking regexp
+       * top/maint.mk (sc_prohibit_xalloc_without_use): Use more complete
+       regexp.  Before, we would give a false-positive (saying alloca.h
+       is included unnecessarily) when the only uses involved omitted symbols.
+
        xalloc.h: use consistent formatting
        * lib/xalloc.h: Move declarations to start in the first column.

diff --git a/top/maint.mk b/top/maint.mk
index 2bdf1eb..e4a6359 100644
--- a/top/maint.mk
+++ b/top/maint.mk
@@ -324,9 +324,15 @@ sc_prohibit_error_without_use:
 # | sort | perl -MRegexp::Assemble -le \
 #  'print Regexp::Assemble->new(file => "/dev/stdin")->as_string'|sed 
's/\?://g'
 # Note this was produced by the above:
-# _xa1 = x(alloc_(oversized|die)|([cz]|2?re)alloc|m(alloc|emdup)|strdup)
-# But we can do better:
-_xa1 = x(alloc_(oversized|die)|([cmz]|2?re)alloc|(mem|str)dup)
+# _xa1 = \
+#x(((2n?)?re|c(har)?|n(re|m)|z)alloc|alloc_(oversized|die)|m(alloc|emdup)|strdup)
+# But we can do better, in at least two ways:
+# 1) take advantage of two "dup"-suffixed strings:
+# x(((2n?)?re|c(har)?|n(re|m)|[mz])alloc|alloc_(oversized|die)|(mem|str)dup)
+# 2) notice that "c(har)?|[mz]" is equivalent to the shorter and more readable
+# "char|[cmz]"
+# x(((2n?)?re|char|n(re|m)|[cmz])alloc|alloc_(oversized|die)|(mem|str)dup)
+_xa1 = x(((2n?)?re|char|n(re|m)|[cmz])alloc|alloc_(oversized|die)|(mem|str)dup)
 _xa2 = X([CZ]|N?M)ALLOC
 sc_prohibit_xalloc_without_use:
        @h='"xalloc.h"' \
--
1.6.6.384.g14e6a


>From d297c3ebfd00294f056a5d35c189da0740716703 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Tue, 5 Jan 2010 15:14:35 +0100
Subject: [PATCH 3/3] maint: remove useless inclusions of "alloca.h"

* lib/getloadavg.c: Remove useless inclusion of "alloca.h".
* lib/readtokens.c: Likewise.
* lib/same.c: Likewise.
* modules/getloadavg (Depends-on): Remove alloca.
* modules/readtokens: Likewise.
* modules/same: Likewise.
---
 ChangeLog          |    8 ++++++++
 lib/getloadavg.c   |    1 -
 lib/readtokens.c   |    2 --
 lib/same.c         |    1 -
 modules/getloadavg |    9 ++++-----
 modules/readtokens |    1 -
 modules/same       |    1 -
 7 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 1313708..ca16e48 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
 2010-01-05  Jim Meyering  <address@hidden>

+       maint: remove useless inclusions of "alloca.h"
+       * lib/getloadavg.c: Remove useless inclusion of "alloca.h".
+       * lib/readtokens.c: Likewise.
+       * lib/same.c: Likewise.
+       * modules/getloadavg (Depends-on): Remove alloca.
+       * modules/readtokens: Likewise.
+       * modules/same: Likewise.
+
        maint.mk: include 4 more function names in alloca.h-checking regexp
        * top/maint.mk (sc_prohibit_xalloc_without_use): Use more complete
        regexp.  Before, we would give a false-positive (saying alloca.h
diff --git a/lib/getloadavg.c b/lib/getloadavg.c
index 4a9ca7f..c6d782b 100644
--- a/lib/getloadavg.c
+++ b/lib/getloadavg.c
@@ -111,7 +111,6 @@
 # include "c-strtod.h"
 # include "cloexec.h"
 # include "intprops.h"
-# include "xalloc.h"

 /* The existing Emacs configuration files define a macro called
    LOAD_AVE_CVT, which accepts a value of type LOAD_AVE_TYPE, and
diff --git a/lib/readtokens.c b/lib/readtokens.c
index e29f20e..8172eb1 100644
--- a/lib/readtokens.c
+++ b/lib/readtokens.c
@@ -31,8 +31,6 @@
 #include <string.h>
 #include <stdbool.h>

-#include "xalloc.h"
-
 #if USE_UNLOCKED_IO
 # include "unlocked-io.h"
 #endif
diff --git a/lib/same.c b/lib/same.c
index 639d6c5..1abc55c 100644
--- a/lib/same.c
+++ b/lib/same.c
@@ -39,7 +39,6 @@
 #include "dirname.h"
 #include "error.h"
 #include "same-inode.h"
-#include "xalloc.h"

 #ifndef MIN
 # define MIN(a, b) ((a) < (b) ? (a) : (b))
diff --git a/modules/getloadavg b/modules/getloadavg
index 2fbeb40..7865b8f 100644
--- a/modules/getloadavg
+++ b/modules/getloadavg
@@ -6,14 +6,13 @@ lib/getloadavg.c
 m4/getloadavg.m4

 Depends-on:
-stdlib
-extensions
-cloexec
-xalloc
 c-strtod
+cloexec
+extensions
+fcntl-safer
 intprops
 stdbool
-fcntl-safer
+stdlib

 configure.ac:
 gl_GETLOADAVG([$gl_source_base])
diff --git a/modules/readtokens b/modules/readtokens
index 9453de9..d593b8f 100644
--- a/modules/readtokens
+++ b/modules/readtokens
@@ -7,7 +7,6 @@ lib/readtokens.c
 m4/readtokens.m4

 Depends-on:
-xalloc
 stdbool

 configure.ac:
diff --git a/modules/same b/modules/same
index a6fc64f..7a9252a 100644
--- a/modules/same
+++ b/modules/same
@@ -8,7 +8,6 @@ lib/same.c
 m4/same.m4

 Depends-on:
-xalloc
 error
 dirname
 same-inode
--
1.6.6.384.g14e6a