argp warnings

[Eric Blake]

There are several problems with the argp module:

$ CFLAGS=-Wall ./gnulib-tool --with-tests --test argp
warning: module argp depends on a module with an incompatible license: dirname
warning: module argp depends on a module with an incompatible license: exit
warning: module argp depends on a module with an incompatible license: exitfail
warning: module argp depends on a module with an incompatible license: xalloc
warning: module argp depends on a module with an incompatible license: xalloc-
die
warning: module argp depends on a module with an incompatible license: xstrndup
...
../../gllib/argp-fmtstream.c: In function `_argp_fmtstream_update':
../../gllib/argp-fmtstream.c:229: warning: subscript has type `char'
../../gllib/argp-fmtstream.c:239: warning: subscript has type `char'
../../gllib/argp-fmtstream.c:251: warning: subscript has type `char'
../../gllib/argp-fmtstream.c:264: warning: subscript has type `char'
...
*** argp.11760  Fri May  8 08:56:27 2009
--- -   Fri May  8 08:56:27 2009
***************
*** 1,4 ****
  Usage: test-argp [-tvCSOlp?V] [-f FILE] [-o[ARG]] [--test] [--file=FILE]
              [--input=FILE] [--verbose] [--cantiga] [--sonet] [--option]
!             [--optional[=ARG]] [--limerick] [--poem] [--help] [--usage]
!             [--version] ARGS...
--- 1,4 ----
  Usage: test-argp [-tvCSOlp?V] [-f FILE] [-o[ARG]] [--test] [--file=FILE]
              [--input=FILE] [--verbose] [--cantiga] [--sonet] [--option]
!             [--optional[=ARG]] [--limerick] [--poem] [--help] [--version]
!             [--usage] ARGS...
*** argp.11760  Fri May  8 08:56:27 2009
--- -   Fri May  8 08:56:27 2009
***************
*** 1,3 ****
  Usage: test-argp [-tvCSOlp?V] [-f FILE] [-o[ARG]] [--test] [--file=FILE]
  [--input=FILE] [--verbose] [--cantiga] [--sonet] [--option] [--optional[=ARG]]
! [--limerick] [--poem] [--help] [--usage] [--version] ARGS...
--- 1,3 ----
  Usage: test-argp [-tvCSOlp?V] [-f FILE] [-o[ARG]] [--test] [--file=FILE]
  [--input=FILE] [--verbose] [--cantiga] [--sonet] [--option] [--optional[=ARG]]
! [--limerick] [--poem] [--help] [--version] [--usage] ARGS...
*** argp.11760  Fri May  8 08:56:27 2009
--- -   Fri May  8 08:56:27 2009
***************
*** 27,34 ****
    -p, --poem                 create a poem
  
    -?, --help                 give this help list
-       --usage                give a short usage message
    -V, --version              print program version
  
  Mandatory or optional arguments to long options are also mandatory or optional
  for any corresponding short options.
--- 27,34 ----
    -p, --poem                 create a poem
  
    -?, --help                 give this help list
    -V, --version              print program version
+       --usage                give a short usage message
  
  Mandatory or optional arguments to long options are also mandatory or optional
  for any corresponding short options.
FAIL: test-argp-2.sh
...


I'm not sure what we should do for the license incompatibilities - if argp is 
truly intended to be a library interface, then it shouldn't use xalloc, but 
should instead check for allocation failures and return failure to the caller.  
Maybe we should just relicense argp to be GPL instead of LGPL?

For the second, this patch fixes the bug of calling isxxx on char (where we are 
triggering undefined behavior if a char is signed and the high bit is set).  OK 
to apply the patch below, or would we rather constrain this to c_isblank (just 
space in tab) instead of being locale-dependent?

I'm not sure what the root cause of the third failure is, or whether it might 
be specific to the fact that I built on cygwin; it is just a re-ordering of 
arguments, but any hints on how to debug before I spend more time looking into 
it would be appreciated.


From: Eric Blake <address@hidden>
Date: Fri, 8 May 2009 08:59:46 -0600
Subject: [PATCH] argp: avoid undefined behavior

* lib/argp-fmtstream.c (weak_alias): Pass correct types to ctype
macros.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog            |   12 +++++++++---
 lib/argp-fmtstream.c |    8 ++++----
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index aef3402..ec807ee 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2009-05-08  Eric Blake  <address@hidden>
+
+       argp: avoid undefined behavior
+       * lib/argp-fmtstream.c (weak_alias): Pass correct types to ctype
+       macros.
+
 2009-05-07  Simon Josefsson  <address@hidden>

        * modules/sys_socket (Makefile.am): Substitute
diff --git a/lib/argp-fmtstream.c b/lib/argp-fmtstream.c
index c89a99c..7e6d9c6 100644
--- a/lib/argp-fmtstream.c
+++ b/lib/argp-fmtstream.c
@@ -226,7 +226,7 @@ __argp_fmtstream_update (argp_fmtstream_t fs)
          int i;

          p = buf + (r + 1 - fs->point_col);
-         while (p >= buf && !isblank (*p))
+         while (p >= buf && !isblank ((unsigned char) *p))
            --p;
          nextline = p + 1;     /* This will begin the next line.  */

@@ -236,7 +236,7 @@ __argp_fmtstream_update (argp_fmtstream_t fs)
              if (p >= buf)
                do
                  --p;
-               while (p >= buf && isblank (*p));
+               while (p >= buf && isblank ((unsigned char) *p));
              nl = p + 1;       /* The newline will replace the first blank. */
            }
          else
@@ -248,7 +248,7 @@ __argp_fmtstream_update (argp_fmtstream_t fs)
              if (p < nl)
                do
                  ++p;
-               while (p < nl && !isblank (*p));
+               while (p < nl && !isblank ((unsigned char) *p));
              if (p == nl)
                {
                  /* It already ends a line.  No fussing required.  */
@@ -261,7 +261,7 @@ __argp_fmtstream_update (argp_fmtstream_t fs)
              /* Swallow separating blanks.  */
              do
                ++p;
-             while (isblank (*p));
+             while (isblank ((unsigned char) *p));
              /* The next line will start here.  */
              nextline = p;
            }
-- 
1.6.2.4