Re: [PATCH] maint.mk: forbid exit(-1)

[Jim Meyering]

Eric Blake wrote:
> Libvirt accidentally had an 'exit (-1)' which got by the syntax
> checker; generally, exiting with 255 is not a wise idea.
>
> * top/maint.mk (sc_prohibit_magic_number_exit): Detect negatives.
...
> diff --git a/top/maint.mk b/top/maint.mk
> index d5af750..b39e9ae 100644
> --- a/top/maint.mk
> +++ b/top/maint.mk
> @@ -353,8 +353,9 @@ sc_prohibit_strncpy:
>  #  | xargs --no-run-if-empty \
>  #      perl -pi -e 's/(^|[^.])\b(exit ?)\(0\)/$1$2(EXIT_SUCCESS)/'
>  sc_prohibit_magic_number_exit:
> -     @prohibit='(^|[^.])\<(usage|exit) ?\([0-9]|\<error ?\([1-9][0-9]*,'     
> \
> -     halt='use EXIT_* values rather than magic number'                       
> \
> +     @prohibit='(^|[^.])\<(usage|exit|error) ?\(-?[0-9]+[,)]'        \
> +     exclude='error ?\(0,'                                           \
> +     halt='use EXIT_* values rather than magic number'               \
>         $(_sc_search_regexp)
>
>  # Using EXIT_SUCCESS as the first argument to error is misleading,

I like the factored form of the "prohibit" regexp,
but this new false-positive in coreutils "make syntax-check"
shows why I had left them separate:

    tests/mkdir/selinux:42:  # For HP-UX 11.23: Unknown error (252)
    maint.mk: use EXIT_* values rather than magic number
    make: *** [sc_prohibit_magic_number_exit] Error 1

It is common enough to find strings matching /error ([^,]*)/
that we should not have to exempt them in every gnulib-using package,
so I've exempted them like this:

>From 231deed98fe9cf23fbe3c9a39f0f1d288a46acd4 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Sun, 5 Aug 2012 11:19:19 +0200
Subject: [PATCH] maint.mk: sc_prohibit_magic_number_exit: avoid new false
 positives

* top/maint.mk (sc_prohibit_magic_number_exit): Also filter out matches
for /error ?([^,]*)/.  This avoids false-positives for strings like
"Unknown error (252)", introduced via commit v0.0-7538-g92875a6.
---
 ChangeLog    | 7 +++++++
 top/maint.mk | 2 +-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index 425c02d..d692d97 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2012-08-05  Jim Meyering  <address@hidden>
+
+       maint.mk: sc_prohibit_magic_number_exit: avoid new false positives
+       * top/maint.mk (sc_prohibit_magic_number_exit): Also filter out matches
+       for /error ?([^,]*)/.  This avoids false-positives for strings like
+       "Unknown error (252)", introduced via commit v0.0-7538-g92875a6.
+
 2012-08-02  Carlo de Falco  <address@hidden>  (tiny change)

        base64: Use extern C scope in header file, for C++.
diff --git a/top/maint.mk b/top/maint.mk
index bdc4502..f42c199 100644
--- a/top/maint.mk
+++ b/top/maint.mk
@@ -354,7 +354,7 @@ sc_prohibit_strncpy:
 #      perl -pi -e 's/(^|[^.])\b(exit ?)\(0\)/$1$2(EXIT_SUCCESS)/'
 sc_prohibit_magic_number_exit:
        @prohibit='(^|[^.])\<(usage|exit|error) ?\(-?[0-9]+[,)]'        \
-       exclude='error ?\(0,'                                           \
+       exclude='error ?\((0,|[^,]*)'                                   \
        halt='use EXIT_* values rather than magic number'               \
          $(_sc_search_regexp)

--
1.7.12.rc1.10.g97c7934