Re: bug#8675: error: token "@" is not valid in preprocessor expressions

[Paul Eggert]

>> If the previous Makefile.in was not based on gnulib 2011-04-03 or newer,
>> we need to do nothing; the problem is already fixed.
> 
> I am not sure what version of gnulib my Makefile was based on.

The previous Emacs trunk commit (i.e., the commit before the one that
caused you a problem) was based on gnulib 2011-05-06.  I
also updated Emacs trunk from gnulib on 2011-04-26, 2011-04-17,
2011-04-10, 2011-04-06, and 2011-04-05.  So if
you've been keeping up to date regularly with Emacs, your
old Makefile was based on a suitably-recent gnulib.

>> If the reporter did "make" in the top-level directory of emacs and it did
>> not rebuild config.status, even after configure changed, it needs to be
>> fixed in emacs.
> 
> I am not sure how to determine if this was the case, since running
> ./autogen.sh, ./configure and make bootstrap fixed the problem.
> 
> Let me know if I can help to further troubleshoot this.

Perhaps you could try checking out the old version, building it, and
then doing a "bzr up", and see whether the problem recurs?

The top-level Emacs directory does have a dependency of config.status
on configure, for what that's worth, so a top-level "make" should
rebuild config.status.

> If the reporter only did "make" in the lib/ or src/ subdirectory and not
> in the top-level directory, then either he needs to change his way of working,
> or a rule like
> 
>   ../config.status : $(srcdir)/../configure
>   ../config.status --recheck
> 
> needs to be added in every subdirectory's Makefile. Automake generated
> Makefiles contain such a rule, so maybe that's what is missing in
> emacs/src/Makefile?

His problem occurred in emacs/lib, and emacs/lib/Makefile is
generated by Automake, so I expect this is not the problem.

Re: proposed new module intprops-test

[Paul Eggert]

Thanks for testing that.  I replicated the problem on Solaris 10
with Sun C 5.11 and Solaris 8 with Sun C 5.8.  The following patches
fixed it for me.  I can't easily test HP-UX and IRIX but I tried to
fix all the problems I could deduce remotely.  Apparently these
compilers mishandle constants like 0u, so I rewrote the tests to
use constant expressions like ((unsigned int) 0) instead.

>From 1039e7e89410a6cd9a68b7dee58571630adc Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Thu, 19 May 2011 01:34:14 -0700
Subject: [PATCH 1/3] intprops: TYPE_IS_INTEGER, TYPE_SIGNED not integer
 constant exprs

* doc/intprops.texi (Integer Type Determination): Fix
documentation for TYPE_IS_INTEGER: it returns an constant
expression, not an integer constant expression.  Fix doc for
TYPE_SIGNED: it returns an integer constant expression only if its
argument is an integer type.  (TYPE_IS_INTEGER is the same, but is
hardly worth documented that way)
---
 ChangeLog |   10 ++
 doc/intprops.texi |7 ---
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 04b2904..86001d5 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2011-05-19  Paul Eggert  
+
+   intprops: TYPE_IS_INTEGER, TYPE_SIGNED not integer constant exprs
+   * doc/intprops.texi (Integer Type Determination): Fix
+   documentation for TYPE_IS_INTEGER: it returns an constant
+   expression, not an integer constant expression.  Fix doc for
+   TYPE_SIGNED: it returns an integer constant expression only if its
+   argument is an integer type.  (TYPE_IS_INTEGER is the same, but is
+   hardly worth documented that way)
+
 2011-05-18  Bruno Haible  
 
strerror_r: Avoid clobbering the strerror buffer when possible.
diff --git a/doc/intprops.texi b/doc/intprops.texi
index e8721d7..190afe4 100644
--- a/doc/intprops.texi
+++ b/doc/intprops.texi
@@ -57,14 +57,15 @@ while the second, for integer types, is easier to use.
 @subsection Integer Type Determination
 
 @findex TYPE_IS_INTEGER
-@code{TYPE_IS_INTEGER (@var{t})} expands to an integer constant
+@code{TYPE_IS_INTEGER (@var{t})} expands to a constant
 expression that is 1 if the arithmetic type @var{t} is a integer type.
 @code{_Bool} counts as an integer type.
 
 @findex TYPE_SIGNED
-@code{TYPE_SIGNED (@var{t})} expands to an integer constant expression
+@code{TYPE_SIGNED (@var{t})} expands to a constant expression
 that is 1 if the arithmetic type @var{t} is a signed integer type or a
-floating type.
+floating type.  If @var{t} is an integer type, @code{TYPE_SIGNED (@var{t})}
+expands to an integer constant expression.
 
 Example usage:
 
-- 
1.7.5.1


>From 726e8f9cfeefa95beef26cb41330880759776968 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Thu, 19 May 2011 01:36:25 -0700
Subject: [PATCH 2/3] intprops: work around C compiler bugs

* lib/intprops.h (INT_MULTIPLY_RANGE_OVERFLOW): Work around compiler
bug in Sun C 5.11 2010/08/13 and other compilers; see
.
---
 ChangeLog  |5 +
 lib/intprops.h |   17 +++--
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 86001d5..d12db03 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
 2011-05-19  Paul Eggert  
 
+   intprops: work around C compiler bugs
+   * lib/intprops.h (INT_MULTIPLY_RANGE_OVERFLOW): Work around compiler
+   bug in Sun C 5.11 2010/08/13 and other compilers; see
+   .
+
intprops: TYPE_IS_INTEGER, TYPE_SIGNED not integer constant exprs
* doc/intprops.texi (Integer Type Determination): Fix
documentation for TYPE_IS_INTEGER: it returns an constant
diff --git a/lib/intprops.h b/lib/intprops.h
index a84bd6a..9107a4b 100644
--- a/lib/intprops.h
+++ b/lib/intprops.h
@@ -179,16 +179,21 @@
: 0 < (a))
 
 /* Return 1 if A * B would overflow in [MIN,MAX] arithmetic.
-   See above for restrictions.  */
+   See above for restrictions.  Avoid && and || as they tickle
+   bugs in Sun C 5.11 2010/08/13 and other compilers; see
+   .  */
 #define INT_MULTIPLY_RANGE_OVERFLOW(a, b, min, max) \
   ((b) < 0  \
? ((a) < 0   \
   ? (a) < (max) / (b)   \
-  : (b) < -1 && (min) / (b) < (a))  \
-   : (0 < (b)   \
-  && ((a) < 0   \
-  ? (a) < (min) / (b)   \
-  : (max) / (b) < (a
+  : (b) == -1   \
+  ? 0   \
+  : (min) / (b) < (a))  \
+   : (b) == 0 

Re: proposed new module intprops-test

[Bruno Haible]

Hi Paul,

>  2011-05-19  Paul Eggert  
>  
> + intprops: work around C compiler bugs
> + * lib/intprops.h (INT_MULTIPLY_RANGE_OVERFLOW): Work around compiler
> + bug in Sun C 5.11 2010/08/13 and other compilers; see
> + .

Thanks, this patch fixed the errors on OSF/1 5.1 and reduced the errors on
Solaris 9 to only one:

cc -O -DHAVE_CONFIG_H -I.  -DGNULIB_STRICT_CHECKING=1  -I. -I.  -I.. -I./..  -I.
./gllib -I./../gllib-g -c test-intprops.c
"test-intprops.c", line 40: non-constant bit-field size
"test-intprops.c", line 40: bit-field size < 0: _gl_verify_error_if_negative
cc: acomp failed for test-intprops.c
*** Error code 2

It has no effect on HP-UX and IRIX. So that bug with && and || is specific
to OSF/1 and Solaris.

> From e9d47071a68819604a698a9ad5f188472e1c3792 Mon Sep 17 00:00:00 2001
> From: Paul Eggert 
> Date: Thu, 19 May 2011 01:43:17 -0700
> Subject: [PATCH 3/3] intprop-tests: port to older and more-pedantic compilers
> 
> * modules/intprops-tests (Files): Add tests/macros.h.
> * tests/test-intprops.c: Include macros.h.
> (TYPE_IS_INTEGER): Use ASSERT, not verify, to test this macro, as
> it's no longer documented to expand to an integer constant expression.
> (TYPE_SIGNED): Use ASSERT, not verify, to test this macro when the
> argument is floating point, as it's no longer documented to expand
> to an integer constant expression in that case.

This part of the patch fixed the build on Solaris 9.

> (UINT_MAX, ULONG_MAX, UINTMAX_MAX): Redefine to work around
> compiler bugs reported by Bruno Haible.  See
> .
> (U0, U1): New constants, to work around the same bugs.  Also,
> in tests, use e.g., "(unsigned int) 39" rather than "39u".

This part of the patch has no effect on HP-UX, IRIX, and neither on
OSF/1 and Solaris where the test already compiles fine. You could just as
well revert it.

Analysis of the HP-UX cc bug: Here are successive simplifications of the
expressions. I annotated them with the "cc -c" result.

===
#include 

#include "intprops.h"
#include "verify.h"

#include 
#include 

#define check_binop(op, a, b, min, max, overflow)  \
  (INT_##op##_RANGE_OVERFLOW (a, b, min, max) == (overflow)\
   && INT_##op##_OVERFLOW (a, b) == (overflow))

/* Error */
verify (check_binop (ADD, UINT_MAX, 1u, 0u, UINT_MAX, true));

/* Error */
verify (INT_ADD_RANGE_OVERFLOW (UINT_MAX, 1u, 0u, UINT_MAX) == true && 
INT_ADD_OVERFLOW (UINT_MAX, 1u) == true);

/* OK */
verify (INT_ADD_RANGE_OVERFLOW (UINT_MAX, 1u, 0u, UINT_MAX) == true);

/* Error */
verify (INT_ADD_OVERFLOW (UINT_MAX, 1u) == true);

/* Error */
verify (_GL_BINARY_OP_OVERFLOW (UINT_MAX, 1u, _GL_ADD_OVERFLOW) == true);

/* Error */
verify (_GL_ADD_OVERFLOW (UINT_MAX, 1u, _GL_INT_MINIMUM (UINT_MAX | 1u), 
_GL_INT_MAXIMUM (UINT_MAX | 1u)) == true);

/* Error */
verify ((_GL_INT_MINIMUM (UINT_MAX | 1u) < 0 ? INT_ADD_RANGE_OVERFLOW 
(UINT_MAX, 1u, _GL_INT_MINIMUM (UINT_MAX | 1u), _GL_INT_MAXIMUM (UINT_MAX | 
1u)) : UINT_MAX < 0 ? 1u <= UINT_MAX + 1u : 1u < 0 ? UINT_MAX <= UINT_MAX + 1u 
: UINT_MAX + 1u < 1u) == true);

/* Error */
verify (_GL_INT_MINIMUM (UINT_MAX | 1u) < 0 ? INT_ADD_RANGE_OVERFLOW (UINT_MAX, 
1u, _GL_INT_MINIMUM (UINT_MAX | 1u), _GL_INT_MAXIMUM (UINT_MAX | 1u)) : 
UINT_MAX < 0 ? 1u <= UINT_MAX + 1u : 1u < 0 ? UINT_MAX <= UINT_MAX + 1u : 
UINT_MAX + 1u < 1u);

/* OK */
verify (UINT_MAX < 0 ? 1u <= UINT_MAX + 1u : 1u < 0 ? UINT_MAX <= UINT_MAX + 1u 
: UINT_MAX + 1u < 1u);

/* Error */
verify (_GL_INT_MINIMUM (UINT_MAX | 1u) >= 0);

/* Error */
verify (INT_ADD_RANGE_OVERFLOW (UINT_MAX, 1u, _GL_INT_MINIMUM (UINT_MAX | 1u), 
_GL_INT_MAXIMUM (UINT_MAX | 1u)));

/* Error */
verify (!INT_ADD_RANGE_OVERFLOW (UINT_MAX, 1u, _GL_INT_MINIMUM (UINT_MAX | 1u), 
_GL_INT_MAXIMUM (UINT_MAX | 1u)));

/* Error */
verify (_GL_INT_MINIMUM (UINT_MAX) >= 0);

/* Error */
verify (_GL_INT_MINIMUM (1u) >= 0);

/* Error */
verify (!_GL_INT_SIGNED (1u));

/* Error */
verify (_GL_INT_CONVERT (1u, 0) >= 0);

/* Error */
verify (1u - 1u + 0 >= 0);

/* Error */
verify (1u - 1u >= 0);

/* Error */
verify (1u + 0 >= 0);

/* OK */
verify (1u >= 0);
===

As you can see, plus and minus expressions of which one operand is unsigned are
not understood by this compiler. It yells
"error 1511: Bit-field size must be a constant."

Writing  (unsigned int) 1  instead of  1u  does not help.

So I think, you will have to use ASSERT here instead of verify.

Analysis of the IRIX cc bug:
===
#include 

#include "intprops.h"
#include "verify.h"

#include 
#include 

#define check_binop(op, a, b, min, max, overflow)  \
  (INT_##op##_RANGE_OVERFLOW (a,

[PATCH] strerror: enforce POSIX ruling on strerror(0)

[Eric Blake]

http://austingroupbugs.net/view.php?id=382 requires that strerror(0)
succeed, but FreeBSD reports "Unknown error: 0" and fails with EINVAL.

* m4/strerror.m4 (gl_FUNC_STRERROR_SEPARATE): Expose BSD bug.
* m4/strerror_r.m4 (gl_FUNC_STRERROR_R): Likewise.
* lib/strerror_r.c (rpl_strerror_r): Work around it.
* doc/posix-functions/strerror.texi (strerror): Document it.
* doc/posix-functions/strerror_r.texi (strerror_r): Likewise.
* tests/test-strerror.c (main): Strengthen test.
* tests/test-strerror_r.c (main): Likewise.

Signed-off-by: Eric Blake 
---
 ChangeLog   |   11 +++
 doc/posix-functions/strerror.texi   |   11 +--
 doc/posix-functions/strerror_r.texi |4 
 lib/strerror_r.c|   16 
 m4/strerror.m4  |   21 ++---
 m4/strerror_r.m4|   14 +++---
 tests/test-strerror.c   |   20 +++-
 tests/test-strerror_r.c |   16 ++--
 8 files changed, 90 insertions(+), 23 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index b21dc1b..8c1200c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2011-05-19  Eric Blake  
+
+   strerror: enforce POSIX ruling on strerror(0)
+   * m4/strerror.m4 (gl_FUNC_STRERROR_SEPARATE): Expose BSD bug.
+   * m4/strerror_r.m4 (gl_FUNC_STRERROR_R): Likewise.
+   * lib/strerror_r.c (rpl_strerror_r): Work around it.
+   * doc/posix-functions/strerror.texi (strerror): Document it.
+   * doc/posix-functions/strerror_r.texi (strerror_r): Likewise.
+   * tests/test-strerror.c (main): Strengthen test.
+   * tests/test-strerror_r.c (main): Likewise.
+
 2011-05-19  Paul Eggert  

intprop-tests: port to older and more-pedantic compilers
diff --git a/doc/posix-functions/strerror.texi 
b/doc/posix-functions/strerror.texi
index 68a98da..6f9519a 100644
--- a/doc/posix-functions/strerror.texi
+++ b/doc/posix-functions/strerror.texi
@@ -13,11 +13,18 @@ strerror
 but not defined by the system, on some platforms:
 OpenBSD 4.0, OSF/1 5.1, NonStop Kernel, Cygwin 1.5.x, mingw.
 @item
+This function reports failure (by setting @code{errno}) for
+@code{strerror(0)}, although POSIX requires this to leave @code{errno}
+unchanged and report success, on some platforms:
+FreeBSD 8.2
+@item
 This function fails to return a string for out-of-range integers on
 some platforms:
 HP-UX 11, IRIX 6.5, Solaris 8.
-(This is not a POSIX violation, but can still cause bugs because most programs
-call @code{strerror} without setting and testing @code{errno}.)
+(Some return NULL which is a POSIX violation, others return the empty
+string which is valid but not as useful); this can still cause bugs
+because most programs call @code{strerror} without setting and testing
+@code{errno}.)
 @end itemize

 Portability problems not fixed by Gnulib:
diff --git a/doc/posix-functions/strerror_r.texi 
b/doc/posix-functions/strerror_r.texi
index bf43164..9d6639e 100644
--- a/doc/posix-functions/strerror_r.texi
+++ b/doc/posix-functions/strerror_r.texi
@@ -37,6 +37,10 @@ strerror_r
 but not defined by the system, on some platforms:
 OpenBSD 4.0, OSF/1 5.1, NonStop Kernel, Cygwin 1.5.x.
 @item
+This function reports failure for @code{strerror_r(0, buf, len)},
+although POSIX requires this to succeed, on some platforms:
+FreeBSD 8.2
+@item
 This function always fails when the third argument is less than 80 on some
 platforms:
 HP-UX 11.31.
diff --git a/lib/strerror_r.c b/lib/strerror_r.c
index 3bba26c..fc0603c 100644
--- a/lib/strerror_r.c
+++ b/lib/strerror_r.c
@@ -436,6 +436,22 @@ strerror_r (int errnum, char *buf, size_t buflen)
 if (ret < 0)
   ret = errno;

+/* FreeBSD rejects 0; see http://austingroupbugs.net/view.php?id=382.  */
+if (errnum == 0 && ret == EINVAL)
+  {
+if (buflen <= strlen ("Success"))
+  {
+ret = ERANGE;
+if (buflen)
+  buf[0] = 0;
+  }
+else
+  {
+ret = 0;
+strcpy (buf, "Success");
+  }
+  }
+
 #elif USE_XPG_STRERROR_R

 {
diff --git a/m4/strerror.m4 b/m4/strerror.m4
index 73d1d54..d891031 100644
--- a/m4/strerror.m4
+++ b/m4/strerror.m4
@@ -1,4 +1,4 @@
-# strerror.m4 serial 9
+# strerror.m4 serial 10
 dnl Copyright (C) 2002, 2007-2011 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -25,19 +25,18 @@ AC_DEFUN([gl_FUNC_STRERROR_SEPARATE],
  [AC_RUN_IFELSE(
 [AC_LANG_PROGRAM(
[[#include 
+ #include 
]],
-   [[return !*strerror (-2);]])],
+   [[int result = 0;
+ if (!*strerror (-2)) result |= 1;
+ errno = 0;
+ if (!*strerror (0)) result |= 2;
+ if (errno) result |= 4;
+ return result;]])],
 [gl_cv_func_working_strerro

[PATCH] strerror: relax test for Solaris

[Eric Blake]

Solaris returns "Error 0" for 0 vs. "Unknown error" for -1; while that
does not quite have the positive connotation that POSIX recommends, it
is distinct enough that we do not need to replace the native strerror
just to pick a better string.

The test is really trying to reject implementations that use the same
string for both 0 and 1 (modulo any %d output of the error number).

* tests/test-strerror.c (main): Permit Solaris behavior.
* tests/test-strerror_r.c (main): Likewise.

Signed-off-by: Eric Blake 
---
 ChangeLog   |4 
 tests/test-strerror.c   |6 +++---
 tests/test-strerror_r.c |6 +++---
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 8c1200c..b90c116 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
 2011-05-19  Eric Blake  

+   strerror: relax test for Solaris
+   * tests/test-strerror.c (main): Permit Solaris behavior.
+   * tests/test-strerror_r.c (main): Likewise.
+
strerror: enforce POSIX ruling on strerror(0)
* m4/strerror.m4 (gl_FUNC_STRERROR_SEPARATE): Expose BSD bug.
* m4/strerror_r.m4 (gl_FUNC_STRERROR_R): Likewise.
diff --git a/tests/test-strerror.c b/tests/test-strerror.c
index 46d339e..03637d7 100644
--- a/tests/test-strerror.c
+++ b/tests/test-strerror.c
@@ -51,15 +51,15 @@ main (void)
   ASSERT (*str);
   ASSERT (errno == 0);

-  /* POSIX requires strerror (0) to succeed; use of "Unknown error" or
- "error 0" does not count as success, but "No error" works.
+  /* POSIX requires strerror (0) to succeed.  Reject use of "Unknown
+ error", but allow "Success", "No error", or even Solaris' "Error
+ 0" which are distinct patterns from true out-of-range strings.
  http://austingroupbugs.net/view.php?id=382  */
   errno = 0;
   str = strerror (0);
   ASSERT (str);
   ASSERT (*str);
   ASSERT (errno == 0);
-  ASSERT (strchr (str, '0') == NULL);
   ASSERT (strstr (str, "nknown") == NULL);

   /* POSIX requires strerror to produce a non-NULL result for all
diff --git a/tests/test-strerror_r.c b/tests/test-strerror_r.c
index 0661bdf..7aad3c7 100644
--- a/tests/test-strerror_r.c
+++ b/tests/test-strerror_r.c
@@ -46,14 +46,14 @@ main (void)
   ASSERT (strerror_r (EOVERFLOW, buf, sizeof (buf)) == 0);
   ASSERT (buf[0] != '\0');

-  /* POSIX requires strerror (0) to succeed; use of "Unknown error" or
- "error 0" does not count as success, but "No error" works.
+  /* POSIX requires strerror (0) to succeed.  Reject use of "Unknown
+ error", but allow "Success", "No error", or even Solaris' "Error
+ 0" which are distinct patterns from true out-of-range strings.
  http://austingroupbugs.net/view.php?id=382  */
   buf[0] = '\0';
   ret = strerror_r (0, buf, sizeof (buf));
   ASSERT (ret == 0);
   ASSERT (buf[0]);
-  ASSERT (strchr (buf, '0') == NULL);
   ASSERT (strstr (buf, "nknown") == NULL);

   /* Test results with out-of-range errnum and enough room.  */
-- 
1.7.4.4

Re: strerror_r on mingw

[Bruno Haible]

> 2011-05-18  Bruno Haible  
> 
>   strerror_r: Avoid clobbering the strerror buffer when possible.
>   * lib/strerror_r.c (strerror_r): Merge the three implementations.
>   Handle gnulib defined errno values here. When strerror() returns NULL
>   or an empty string, return EINVAL.
>   * lib/strerror.c (strerror): Always call strerror_r. Don't handle
>   gnulib defined errno values here.
>   * modules/strerror (Depends-on): Add verify, strerror_r-posix.

Oops, this led to a test failure on mingw. EXTEND_STRERROR_R was not defined
on mingw, because the function strerror_r does not exist there in the first
place. This should be more reliable:


2011-05-19  Bruno Haible  

strerror_r: Fix test failure on mingw.
* m4/strerror_r.m4 (gl_FUNC_STRERROR_R): Don't define
EXTEND_STRERROR_R.
* lib/strerror_r.c (strerror_r): Test the various GNULIB_defined_*
macros from errno.in.h instead.

--- m4/strerror_r.m4.orig   Thu May 19 20:39:58 2011
+++ m4/strerror_r.m4Thu May 19 20:39:16 2011
@@ -1,4 +1,4 @@
-# strerror_r.m4 serial 4
+# strerror_r.m4 serial 5
 dnl Copyright (C) 2002, 2007-2011 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -93,9 +93,6 @@
   dnl The system's strerror_r() cannot know about the new errno values we
   dnl add to . Replace it.
   REPLACE_STRERROR_R=1
-  AC_DEFINE([EXTEND_STRERROR_R], [1],
-[Define to 1 if strerror_r needs to be extended so that it handles the
- extra errno values.])
 fi
   fi
   if test $HAVE_DECL_STRERROR_R = 0 || test $REPLACE_STRERROR_R = 1; then
--- lib/strerror_r.c.orig   Thu May 19 20:39:58 2011
+++ lib/strerror_r.cThu May 19 20:38:31 2011
@@ -64,7 +64,19 @@
 strerror_r (int errnum, char *buf, size_t buflen)
 #undef strerror_r
 {
-#if EXTEND_STRERROR_R
+#if GNULIB_defined_ETXTBSY \
+|| GNULIB_defined_ESOCK \
+|| GNULIB_defined_ENOMSG \
+|| GNULIB_defined_EIDRM \
+|| GNULIB_defined_ENOLINK \
+|| GNULIB_defined_EPROTO \
+|| GNULIB_defined_EMULTIHOP \
+|| GNULIB_defined_EBADMSG \
+|| GNULIB_defined_EOVERFLOW \
+|| GNULIB_defined_ENOTSUP \
+|| GNULIB_defined_ESTALE \
+|| GNULIB_defined_EDQUOT \
+|| GNULIB_defined_ECANCELED
   {
 char const *msg = NULL;
 /* These error messages are taken from glibc/sysdeps/gnu/errlist.c.  */

-- 
In memoriam Anne Boleyn 

Re: perror bug

[Bruno Haible]

> >   1) The strerror_r replacement, when EXTEND_STRERROR_R is defined,
> >      clobbers the strerror function's buffer, which it shouldn't.

Here's a followup that tries harder to clobber the strerror function's
buffer. It still is not possible on Solaris <= 9 in 64-bit mode, because
that libc does not export 'sys_nerr'.


2011-05-19  Bruno Haible  

strerror_r: Avoid clobbering the strerror buffer when possible.
* lib/strerror.c: Define _NETBSD_SOURCE. Include .
(sys_nerr, sys_errlist): New declarations.
(strerror_r): Be careful not to clobber the strerror buffer on NetBSD,
HP-UX, native Win32, IRIX, and 32-bit Solaris.
* m4/strerror_r.m4 (gl_PREREQ_STRERROR_R): Test whether catgets exists.

--- lib/strerror_r.c.orig   Thu May 19 20:50:20 2011
+++ lib/strerror_r.cThu May 19 20:49:54 2011
@@ -19,6 +19,9 @@
 
 #include 
 
+/* Enable declaration of sys_nerr and sys_errlist in  on NetBSD.  */
+#define _NETBSD_SOURCE 1
+
 /* Specification.  */
 #include 
 
@@ -46,17 +49,45 @@
 
 #else /* (__GLIBC__ >= 2 || defined __UCLIBC__ ? !HAVE___XPG_STRERROR_R : 
!HAVE_DECL_STRERROR_R) */
 
-# include "glthread/lock.h"
-
-/* Use strerror(), with locking.  */
+/* Use the system's strerror().  */
 # undef strerror
 
 # define USE_SYSTEM_STRERROR 1
 
+# if defined __NetBSD__ || defined __hpux || ((defined _WIN32 || defined 
__WIN32__) && !defined __CYGWIN__) || defined __sgi || (defined __sun && 
!defined _LP64)
+
+/* No locking needed.  */
+
+/* Get catgets internationalization functions.  */
+#  if HAVE_CATGETS
+#   include 
+#  endif
+
+/* Get sys_nerr, sys_errlist on HP-UX (otherwise only declared in C++ mode).
+   Get sys_nerr, sys_errlist on IRIX (otherwise only declared with _SGIAPI).  
*/
+#  if defined __hpux || defined __sgi
+extern int sys_nerr;
+extern char *sys_errlist[];
+#  endif
+
+/* Get sys_nerr on Solaris.  */
+#  if defined __sun && !defined _LP64
+extern int sys_nerr;
+#  endif
+
+/* Get sys_nerr, sys_errlist on native Windows.  */
+#  include 
+
+# else
+
+#  include "glthread/lock.h"
+
 /* This lock protects the buffer returned by strerror().  We assume that
no other uses of strerror() exist in the program.  */
 gl_lock_define_initialized(static, strerror_lock)
 
+# endif
+
 #endif
 
 
@@ -476,6 +507,87 @@
 
 #else /* USE_SYSTEM_STRERROR */
 
+/* Try to do what strerror (errnum) does, but without clobbering the
+   buffer used by strerror().  */
+
+# if defined __NetBSD__ || defined __hpux || ((defined _WIN32 || defined 
__WIN32__) && !defined __CYGWIN__) /* NetBSD, HP-UX, native Win32 */
+
+/* NetBSD:sys_nerr, sys_errlist are declared through _NETBSD_SOURCE
+  and  above.
+   HP-UX: sys_nerr, sys_errlist are declared explicitly above.
+   native Win32:  sys_nerr, sys_errlist are declared in .  */
+if (errnum >= 0 && errnum < sys_nerr)
+  {
+#  if HAVE_CATGETS && (defined __NetBSD__ || defined __hpux)
+int saved_errno = errno;
+#   if defined __NetBSD__
+nl_catd catd = catopen ("libc", NL_CAT_LOCALE);
+const char *errmsg =
+  (catd != (nl_catd)-1
+   ? catgets (catd, 1, errnum, sys_errlist[errnum])
+   : sys_errlist[errnum]);
+#   endif
+#   if defined __hpux
+nl_catd catd = catopen ("perror", NL_CAT_LOCALE);
+const char *errmsg =
+  (catd != (nl_catd)-1
+   ? catgets (catd, 1, 1 + errnum, sys_errlist[errnum])
+   : sys_errlist[errnum]);
+#   endif
+#  else
+const char *errmsg = sys_errlist[errnum];
+#  endif
+if (errmsg == NULL || *errmsg == '\0')
+  ret = EINVAL;
+else
+  {
+size_t len = strlen (errmsg);
+
+if (len < buflen)
+  {
+memcpy (buf, errmsg, len + 1);
+ret = 0;
+  }
+else
+  ret = ERANGE;
+  }
+#  if HAVE_CATGETS && (defined __NetBSD__ || defined __hpux)
+if (catd != (nl_catd)-1)
+  catclose (catd);
+errno = saved_errno;
+#  endif
+  }
+else
+  ret = EINVAL;
+
+# elif defined __sgi || (defined __sun && !defined _LP64) /* IRIX, Solaris <= 
9 32-bit */
+
+/* For a valid error number, the system's strerror() function returns
+   a pointer to a not copied string, not to a buffer.  */
+if (errnum >= 0 && errnum < sys_nerr)
+  {
+char *errmsg = strerror (errnum);
+
+if (errmsg == NULL || *errmsg == '\0')
+  ret = EINVAL;
+else
+  {
+size_t len = strlen (errmsg);
+
+if (len < buflen)
+  {
+memcpy (buf, errmsg, len + 1);
+ret = 0;
+  }
+else
+  ret = ERANGE;
+  }
+  }
+else
+  ret = EINVAL;
+
+# else
+
 gl_lock_lock (strerror_lock);
 
 {
@@ -502,6 +614,8 @@
 
 gl_lock_unlock (strerror_lock);
 
+# endif
+
 #endif
 

Re: perror bug

[Bruno Haible]

> +# elif defined __sgi || (defined __sun && !defined _LP64) /* IRIX, Solaris 
> <= 9 32-bit */
> +
> +    /* For a valid error number, the system's strerror() function returns
> +       a pointer to a not copied string, not to a buffer.  */

Just for reference: How to find out whether a closed-source strerror()
function uses a buffer or not. Run this program and observe whether
all three addresses are the same or not.

===
#include 
#include 
#include 

int main ()
{
  const char *msg1;
  const char *msg2;
  const char *msg3;

  msg1 = strerror (ENOENT);
  printf ("msg1 before: %p %s\n", msg1, msg1);
  msg2 = strerror (ENOTDIR);
  printf ("msg2 before: %p %s\n", msg2, msg2);
  msg3 = strerror (0);
  printf ("msg3 before: %p %s\n", msg3, msg3);

  strerror (ENOENT);
  strerror (ENOTDIR);
  strerror (0);

  printf ("msg1 after:  %s\n", msg1);
  printf ("msg2 after:  %s\n", msg2);
  printf ("msg3 after:  %s\n", msg3);

  return 0;
}
===
-- 
In memoriam Anne Boleyn 

[PATCH] strerror_r: fix on newer cygwin

[Eric Blake]

* lib/strerror_r.c (rpl_strerror_r): Cygwin now has
__xpg_strerror_r, use it.

Signed-off-by: Eric Blake 
---

Without this, recent test improvements were exposing failures
of gnulib's replacement on cygwin.

 ChangeLog|6 ++
 lib/strerror_r.c |   12 ++--
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 4e22ecb..f5e455b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2011-05-19  Eric Blake  
+
+   strerror_r: fix on newer cygwin
+   * lib/strerror_r.c (rpl_strerror_r): Cygwin now has
+   __xpg_strerror_r, use it.
+
 2011-05-19  Bruno Haible  

strerror_r: Avoid clobbering the strerror buffer when possible.
diff --git a/lib/strerror_r.c b/lib/strerror_r.c
index c4aa5d9..fe1185b 100644
--- a/lib/strerror_r.c
+++ b/lib/strerror_r.c
@@ -34,7 +34,11 @@
 # endif


-#if HAVE_DECL_STRERROR_R && !(__GLIBC__ >= 2 || defined __UCLIBC__)
+#if (__GLIBC__ >= 2 || defined __UCLIBC__ || defined __CYGWIN__) && 
HAVE___XPG_STRERROR_R /* glibc >= 2.3.4, cygwin >= 1.7.9 */
+
+# define USE_XPG_STRERROR_R 1
+
+#elif HAVE_DECL_STRERROR_R && !(__GLIBC__ >= 2 || defined __UCLIBC__)

 /* The system's strerror_r function is OK, except that its third argument
is 'int', not 'size_t', or its return type is wrong.  */
@@ -43,10 +47,6 @@

 # define USE_SYSTEM_STRERROR_R 1

-#elif (__GLIBC__ >= 2 || defined __UCLIBC__) && HAVE___XPG_STRERROR_R /* glibc 
>= 2.3.4 */
-
-# define USE_XPG_STRERROR_R 1
-
 #else /* (__GLIBC__ >= 2 || defined __UCLIBC__ ? !HAVE___XPG_STRERROR_R : 
!HAVE_DECL_STRERROR_R) */

 /* Use the system's strerror().  */
@@ -446,7 +446,7 @@ strerror_r (int errnum, char *buf, size_t buflen)
 ret = strerror_r (errnum, buf, buflen);
 }
 # elif defined __CYGWIN__
-/* Cygwin only provides the glibc interface, is thread-safe, and
+/* Cygwin 1.7.8 only provides the glibc interface, is thread-safe, and
always succeeds (although it may truncate). */
 strerror_r (errnum, buf, buflen);
 ret = 0;
-- 
1.7.4.4

Re: perror bug

[Bruno Haible]

On 2011-14-03 I wrote in
:
>   2) The perror replacement uses strerror, thus clobbering the strerror
>  buffer.
>   3) On Cygwin, perror clobbers the strerror buffer.
> 
> The fix for 2) should be to change lib/perror.c to call strerror_r.
> The fix for 3) should be to change m4/perror.m4 to enable the replacement
> on Cygwin.

Here comes part 2. Eric, I leave part 3 to you.


2011-05-19  Bruno Haible  

perror: Avoid clobbering the strerror buffer when possible.
* lib/strerror-impl.h: New file, extracted from lib/strerror.c.
* lib/strerror.c: Include it.
* modules/strerror (Files): Add lib/strerror-impl.h.
* lib/perror.c: Include , intprops.h, verify.h.
(my_strerror): New function, defined through lib/strerror-impl.h.
(perror): Use it instead of strerror.
* modules/perror (Files): Add lib/strerror-impl.h.
(Depends-on): Remove strerror. Add intprops, verify, strerror_r-posix.

= lib/strerror-impl.h =
/* strerror-impl.h --- Implementation of POSIX compatible strerror() function.

   Copyright (C) 2007-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 .  */

#ifdef STATIC
STATIC
#endif
char *
strerror (int n)
{
  static char buf[256];

  int ret = strerror_r (n, buf, sizeof (buf));

  if (ret == 0)
return buf;

  if (ret == ERANGE)
/* If this happens, increase the size of buf.  */
abort ();

  {
static char const fmt[] = "Unknown error (%d)";
verify (sizeof (buf) >= sizeof (fmt) + INT_STRLEN_BOUND (n));
sprintf (buf, fmt, n);
return buf;
  }
}
===
--- lib/perror.c.orig   Thu May 19 21:44:53 2011
+++ lib/perror.cThu May 19 21:40:43 2011
@@ -21,12 +21,26 @@
 #include 
 
 #include 
+#include 
 #include 
 
+#include "intprops.h"
+#include "verify.h"
+
+/* Use the system functions, not the gnulib overrides in this file.  */
+#undef sprintf
+
+/* my_strerror (errnum) is equivalent to strerror (errnum).
+   But it uses its own buffer, not the one from strerror().  */
+#define STATIC static
+#undef strerror
+#define strerror my_strerror
+#include "strerror-impl.h"
+
 void
 perror (const char *string)
 {
-  const char *errno_description = strerror (errno);
+  const char *errno_description = my_strerror (errno);
 
   if (string != NULL && *string != '\0')
 fprintf (stderr, "%s: %s\n", string, errno_description);
--- lib/strerror.c.orig Thu May 19 21:44:53 2011
+++ lib/strerror.c  Thu May 19 21:22:27 2011
@@ -32,26 +32,6 @@
 /* Use the system functions, not the gnulib overrides in this file.  */
 # undef sprintf
 
-char *
-strerror (int n)
-{
-  static char buf[256];
-
-  int ret = strerror_r (n, buf, sizeof (buf));
-
-  if (ret == 0)
-return buf;
-
-  if (ret == ERANGE)
-/* If this happens, increase the size of buf.  */
-abort ();
-
-  {
-static char const fmt[] = "Unknown error (%d)";
-verify (sizeof (buf) >= sizeof (fmt) + INT_STRLEN_BOUND (n));
-sprintf (buf, fmt, n);
-return buf;
-  }
-}
+# include "strerror-impl.h"
 
 #endif
--- modules/perror.orig Thu May 19 21:44:53 2011
+++ modules/perror  Thu May 19 21:23:39 2011
@@ -3,12 +3,15 @@
 
 Files:
 lib/perror.c
+lib/strerror-impl.h
 m4/perror.m4
 
 Depends-on:
 stdio
-errno   [test $REPLACE_PERROR = 1]
-strerror[test $REPLACE_PERROR = 1]
+errno[test $REPLACE_PERROR = 1]
+intprops [test $REPLACE_PERROR = 1]
+verify   [test $REPLACE_PERROR = 1]
+strerror_r-posix [test $REPLACE_PERROR = 1]
 
 configure.ac:
 gl_FUNC_PERROR
--- modules/strerror.orig   Thu May 19 21:44:53 2011
+++ modules/strerrorThu May 19 21:19:22 2011
@@ -3,6 +3,7 @@
 
 Files:
 lib/strerror.c
+lib/strerror-impl.h
 m4/strerror.m4
 
 Depends-on:
-- 
In memoriam Anne Boleyn 

[PATCH] strerror_r: guarantee unchanged errno

[Eric Blake]

POSIX guarantees that strerror doesn't change errno on success,
and since we implement strerror by using strerror_r, it makes
sense to make the same guarantee for strerror_r (rather, going
one step further to say that sterror_r does not corrupt errno
even on failure, since it returns an error value explicitly).

See also http://austingroupbugs.net/view.php?id=447

* lib/strerror_r.c (strerror_r): Guarantee unchanged errno.
* lib/strerror.c (strerror): Set errno to match strerror_r
failure.
* tests/test-strerror_r.c (main): Enhance test.

Signed-off-by: Eric Blake 
---

Bruno's code was doing part of the effort for NetBSD, but
forgot that other calls outside the protection for catgets
(such as strcpy) can also corrupt errno; also, preserving
errno is important when replacing sterror_r on platforms
that return -1.

I'm also opening up a POSIX bug for the fact that strcpy()
and friend are not documented as leaving errno unchanged,
as it would have been easier to write the test by assuming
that string manipulation doesn't interfere with errno.

 ChangeLog   |6 ++
 lib/strerror.c  |1 +
 lib/strerror_r.c|4 ++--
 tests/test-strerror_r.c |   14 ++
 4 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index f5e455b..0c15e9c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
 2011-05-19  Eric Blake  

+   strerror_r: guarantee unchanged errno
+   * lib/strerror_r.c (strerror_r): Guarantee unchanged errno.
+   * lib/strerror.c (strerror): Set errno to match strerror_r
+   failure.
+   * tests/test-strerror_r.c (main): Enhance test.
+
strerror_r: fix on newer cygwin
* lib/strerror_r.c (rpl_strerror_r): Cygwin now has
__xpg_strerror_r, use it.
diff --git a/lib/strerror.c b/lib/strerror.c
index 6137552..ee7e088 100644
--- a/lib/strerror.c
+++ b/lib/strerror.c
@@ -50,6 +50,7 @@ strerror (int n)
 static char const fmt[] = "Unknown error (%d)";
 verify (sizeof (buf) >= sizeof (fmt) + INT_STRLEN_BOUND (n));
 sprintf (buf, fmt, n);
+errno = ret;
 return buf;
   }
 }
diff --git a/lib/strerror_r.c b/lib/strerror_r.c
index fe1185b..0f05b94 100644
--- a/lib/strerror_r.c
+++ b/lib/strerror_r.c
@@ -418,6 +418,7 @@ strerror_r (int errnum, char *buf, size_t buflen)

   {
 int ret;
+int saved_errno = errno;

 #if USE_SYSTEM_STRERROR_R

@@ -519,7 +520,6 @@ strerror_r (int errnum, char *buf, size_t buflen)
 if (errnum >= 0 && errnum < sys_nerr)
   {
 #  if HAVE_CATGETS && (defined __NetBSD__ || defined __hpux)
-int saved_errno = errno;
 #   if defined __NetBSD__
 nl_catd catd = catopen ("libc", NL_CAT_LOCALE);
 const char *errmsg =
@@ -554,7 +554,6 @@ strerror_r (int errnum, char *buf, size_t buflen)
 #  if HAVE_CATGETS && (defined __NetBSD__ || defined __hpux)
 if (catd != (nl_catd)-1)
   catclose (catd);
-errno = saved_errno;
 #  endif
   }
 else
@@ -618,6 +617,7 @@ strerror_r (int errnum, char *buf, size_t buflen)

 #endif

+errno = saved_errno;
 return ret;
   }
 }
diff --git a/tests/test-strerror_r.c b/tests/test-strerror_r.c
index 7aad3c7..4828767 100644
--- a/tests/test-strerror_r.c
+++ b/tests/test-strerror_r.c
@@ -34,35 +34,45 @@ main (void)

   /* Test results with valid errnum and enough room.  */

+  errno = 0;
   buf[0] = '\0';
   ASSERT (strerror_r (EACCES, buf, sizeof (buf)) == 0);
   ASSERT (buf[0] != '\0');
+  ASSERT (errno == 0);

+  errno = 0;
   buf[0] = '\0';
   ASSERT (strerror_r (ETIMEDOUT, buf, sizeof (buf)) == 0);
   ASSERT (buf[0] != '\0');
+  ASSERT (errno == 0);

+  errno = 0;
   buf[0] = '\0';
   ASSERT (strerror_r (EOVERFLOW, buf, sizeof (buf)) == 0);
   ASSERT (buf[0] != '\0');
+  ASSERT (errno == 0);

   /* POSIX requires strerror (0) to succeed.  Reject use of "Unknown
  error", but allow "Success", "No error", or even Solaris' "Error
  0" which are distinct patterns from true out-of-range strings.
  http://austingroupbugs.net/view.php?id=382  */
+  errno = 0;
   buf[0] = '\0';
   ret = strerror_r (0, buf, sizeof (buf));
   ASSERT (ret == 0);
   ASSERT (buf[0]);
+  ASSERT (errno == 0);
   ASSERT (strstr (buf, "nknown") == NULL);

   /* Test results with out-of-range errnum and enough room.  */

+  errno = 0;
   buf[0] = '^';
   ret = strerror_r (-3, buf, sizeof (buf));
   ASSERT (ret == 0 || ret == EINVAL);
   if (ret == 0)
 ASSERT (buf[0] != '^');
+  ASSERT (errno == 0);

   /* Test results with a too small buffer.  */

@@ -74,7 +84,9 @@ main (void)
 for (i = 0; i <= len; i++)
   {
 strcpy (buf, "BADFACE");
+errno = 0;
 ret = strerror_r (EACCES, buf, i);
+ASSERT (errno == 0);
 if (ret == 0)
   {
 /* Truncated result.  POSIX allows this, and it actually
@@ -90,8 +102,10 @@ main (void)
   }

 strcpy (buf, "BADFACE");
+errno = 0;
 ret = strerror_r (EACCES, buf, len + 1);
  

Re: [PATCH] strerror_r: fix on newer cygwin

[Bruno Haible]

Eric Blake wrote:
> --- a/lib/strerror_r.c
> +++ b/lib/strerror_r.c
> @@ -34,7 +34,11 @@
>  # endif
> 
> 
> -#if HAVE_DECL_STRERROR_R && !(__GLIBC__ >= 2 || defined __UCLIBC__)
> +#if (__GLIBC__ >= 2 || defined __UCLIBC__ || defined __CYGWIN__) && 
> HAVE___XPG_STRERROR_R /* glibc >= 2.3.4, cygwin >= 1.7.9 */
> +
> +# define USE_XPG_STRERROR_R 1
> +
> +#elif HAVE_DECL_STRERROR_R && !(__GLIBC__ >= 2 || defined __UCLIBC__)
> 
>  /* The system's strerror_r function is OK, except that its third argument
> is 'int', not 'size_t', or its return type is wrong.  */
> @@ -43,10 +47,6 @@
> 
>  # define USE_SYSTEM_STRERROR_R 1
> 
> -#elif (__GLIBC__ >= 2 || defined __UCLIBC__) && HAVE___XPG_STRERROR_R /* 
> glibc >= 2.3.4 */
> -
> -# define USE_XPG_STRERROR_R 1
> -
>  #else /* (__GLIBC__ >= 2 || defined __UCLIBC__ ? !HAVE___XPG_STRERROR_R : 
> !HAVE_DECL_STRERROR_R) */
> 
>  /* Use the system's strerror().  */

For consistency, I prefer to have the same order of #ifs in the function as
at the top of the file, so I'm applying this:


2011-05-19  Bruno Haible  

strerror_r: Reorder #if blocks.
* lib/strerror_r.c (strerror_r): Reorder conditionals in the function
for consistency with the previous commit.

--- lib/strerror_r.c.orig   Thu May 19 21:59:21 2011
+++ lib/strerror_r.cThu May 19 21:56:04 2011
@@ -419,7 +419,17 @@
   {
 int ret;
 
-#if USE_SYSTEM_STRERROR_R
+#if USE_XPG_STRERROR_R
+
+{
+  extern int __xpg_strerror_r (int errnum, char *buf, size_t buflen);
+
+  ret = __xpg_strerror_r (errnum, buf, buflen);
+  if (ret < 0)
+ret = errno;
+}
+
+#elif USE_SYSTEM_STRERROR_R
 
 if (buflen > INT_MAX)
   buflen = INT_MAX;
@@ -495,16 +505,6 @@
   }
   }
 
-#elif USE_XPG_STRERROR_R
-
-{
-  extern int __xpg_strerror_r (int errnum, char *buf, size_t buflen);
-
-  ret = __xpg_strerror_r (errnum, buf, buflen);
-  if (ret < 0)
-ret = errno;
-}
-
 #else /* USE_SYSTEM_STRERROR */
 
 /* Try to do what strerror (errnum) does, but without clobbering the

-- 
In memoriam Anne Boleyn 

Re: [PATCH] strerror_r: fix on newer cygwin

[Bruno Haible]

Hi Eric,

> Without this, recent test improvements were exposing failures
> of gnulib's replacement on cygwin.

Which failures were this? If I understand it correctly, a gnulib strerror_r
replacement that does not use __xpg_strerror_r on Cygwin 1.7.9 should be
identical to the one built on Cygwin <= 1.7.8. But a testdir created with

$ ./gnulib-tool --create-testdir --dir=/tmp/testdir --with-tests strerror perror

and built on Cygwin 1.7.5 passes all tests.

Bruno
-- 
In memoriam Anne Boleyn 

Re: [PATCH] strerror_r: fix on newer cygwin

[Eric Blake]

On 05/19/2011 02:12 PM, Bruno Haible wrote:
> Hi Eric,
> 
>> Without this, recent test improvements were exposing failures
>> of gnulib's replacement on cygwin.
> 
> Which failures were this? If I understand it correctly, a gnulib strerror_r
> replacement that does not use __xpg_strerror_r on Cygwin 1.7.9 should be
> identical to the one built on Cygwin <= 1.7.8. But a testdir created with
> 
> $ ./gnulib-tool --create-testdir --dir=/tmp/testdir --with-tests strerror 
> perror
> 
> and built on Cygwin 1.7.5 passes all tests.

Cygwin 1.7.8 tried (but failed) to introduce __xpg_strerror_r; it
actually got added in 1.7.9.  But in the process of adding
__xpg_sterror_r, the original strerror_r was fixed to align more closely
to glibc behavior.  Thus, it was changed so that strerror_r(EACCES, buf,
2) no longer overwrites buf with a (truncated) message to buf, so the
caller instead has to manually copy the static string result over to buf.

When testing at commit d118714, I got:

../../gltests/test-strerror_r.c:82: assertion failed
/bin/sh: line 5:  2060 Aborted (core dumped)
EXEEXT='.exe' srcdir='../../gltests' MAKE='make' ${dir}$tst
FAIL: test-strerror_r.exe

at that point, line 82 was:

82  ASSERT ((strcmp (buf, "BADFACE") == 0) == (i == 0));

with i at 1.

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] strerror_r: guarantee unchanged errno

[Bruno Haible]

Hi Eric,

> * lib/strerror_r.c (strerror_r): Guarantee unchanged errno.
>
> ... preserving errno is important when replacing sterror_r on platforms
> that return -1.

Makes sense, yes. 

> --- a/lib/strerror.c
> +++ b/lib/strerror.c
> @@ -50,6 +50,7 @@ strerror (int n)
>  static char const fmt[] = "Unknown error (%d)";
>  verify (sizeof (buf) >= sizeof (fmt) + INT_STRLEN_BOUND (n));
>  sprintf (buf, fmt, n);
> +errno = ret;
>  return buf;
>}
>  }

It's surprising. But indeed, POSIX says:
  "Upon completion, whether successful or not, strerror() shall return
   a pointer to the generated message string."
So we are free to set errno even when returning a string.

Bruno
-- 
In memoriam Anne Boleyn 

strerror translations [was: perror bug]

[Eric Blake]

On 05/18/2011 09:27 PM, Bruno Haible wrote:
>   int
>   strerror_r (int errnum, char *buf, size_t buflen)
> ! #undef strerror_r
>   {
> ! #if EXTEND_STRERROR_R
> !   {
> ! char const *msg = NULL;
> ! /* These error messages are taken from glibc/sysdeps/gnu/errlist.c.  */
> ! switch (errnum)
> !   {
> ! # if GNULIB_defined_ETXTBSY
> !   case ETXTBSY:
> ! msg = "Text file busy";

Should we be marking all of these msg strings for translation?  It would
be a bit disconcerting to have some, but not all, strerror messages
translated on a platform where we are augmenting the existing system
errno values.

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] strerror_r: fix on newer cygwin

[Bruno Haible]

Hi Eric,

> in the process of adding
> __xpg_sterror_r, the original strerror_r was fixed to align more closely
> to glibc behavior.  Thus, it was changed so that strerror_r(EACCES, buf,
> 2) no longer overwrites buf with a (truncated) message to buf, so the
> caller instead has to manually copy the static string result over to buf.
> 
> When testing at commit d118714, I got:
> 
> ../../gltests/test-strerror_r.c:82: assertion failed
> /bin/sh: line 5:  2060 Aborted (core dumped)
> EXEEXT='.exe' srcdir='../../gltests' MAKE='make' ${dir}$tst
> FAIL: test-strerror_r.exe
> 
> at that point, line 82 was:
> 
> 82ASSERT ((strcmp (buf, "BADFACE") == 0) == (i == 0));
> 
> with i at 1.

Thanks for explaining. But that means that a binary built with Cygwin
1.7.5 will still fail when run on Cygwin >= 1.7.9. The complete fix
ought to look something like this, I guess?


2011-05-19  Bruno Haible  

strerror_r: Work around strerror_r() change in Cygwin 1.7.9.
* lib/strerror_r.c (strerror_r) [CYGWIN]: Recognize when the system's
strerror_r() returned without filling the buffer.
Reported by Eric Blake.

--- lib/strerror_r.c.orig   Thu May 19 22:48:15 2011
+++ lib/strerror_r.cThu May 19 22:48:05 2011
@@ -456,10 +456,34 @@
 ret = strerror_r (errnum, buf, buflen);
 }
 # elif defined __CYGWIN__
-/* Cygwin 1.7.8 only provides the glibc interface, is thread-safe, and
-   always succeeds (although it may truncate). */
-strerror_r (errnum, buf, buflen);
-ret = 0;
+/* Cygwin <= 1.7.8 only provides the glibc interface, is thread-safe, and
+   always succeeds (although it may truncate).  In Cygwin >= 1.7.9, instead
+   of truncating, it leaves the buffer untouched.  */
+{
+  char stackbuf[256];
+
+  if (buflen < sizeof (stackbuf))
+{
+  size_t len;
+
+  stackbuf[0] = '\0'; /* in case strerror_r does nothing */
+  strerror_r (errnum, stackbuf, sizeof (stackbuf));
+  len = strlen (stackbuf);
+  if (len < buflen)
+{
+  memcpy (buf, stackbuf, len + 1);
+  ret = 0;
+}
+  else
+ret = ERANGE;
+}
+  else
+{
+  buf[0] = '\0'; /* in case strerror_r does nothing */
+  strerror_r (errnum, buf, buflen);
+  ret = 0;
+}
+}
 # else
 ret = strerror_r (errnum, buf, buflen);
 # endif

-- 
In memoriam Anne Boleyn 

gnulib translations (was: strerror translations)

[Bruno Haible]

Hi Eric,

> Should we be marking all of these msg strings for translation?  It would
> be a bit disconcerting to have some, but not all, strerror messages
> translated on a platform where we are augmenting the existing system
> errno values.

If this is done, the translators should be made aware that they can fetch
the translations from the libc PO file.

But I'm a bit confused about the state of the gnulib translations:

  - I don't know how many people are following the methodology mentioned in
the gnulib manual
.
  - This mail is still unanswered:
.

I'd say, before adding many new strings for the translators to process, we
should make sure that we are making good use of the work the translators
are producing. Any volunteer?

Bruno
-- 
In memoriam Anne Boleyn 

Re: [PATCH] strerror_r: fix on newer cygwin

[Eric Blake]

On 05/19/2011 02:51 PM, Bruno Haible wrote:
>> in the process of adding
>> __xpg_sterror_r, the original strerror_r was fixed to align more closely
>> to glibc behavior.  Thus, it was changed so that strerror_r(EACCES, buf,
>> 2) no longer overwrites buf with a (truncated) message to buf, so the
>> caller instead has to manually copy the static string result over to buf.
>>
> Thanks for explaining. But that means that a binary built with Cygwin
> 1.7.5 will still fail when run on Cygwin >= 1.7.9. The complete fix
> ought to look something like this, I guess?

Indeed, and good catch.

> +++ lib/strerror_r.c  Thu May 19 22:48:05 2011
> @@ -456,10 +456,34 @@
>  ret = strerror_r (errnum, buf, buflen);
>  }
>  # elif defined __CYGWIN__
> -/* Cygwin 1.7.8 only provides the glibc interface, is thread-safe, and
> -   always succeeds (although it may truncate). */
> -strerror_r (errnum, buf, buflen);
> -ret = 0;
> +/* Cygwin <= 1.7.8 only provides the glibc interface, is thread-safe, and
> +   always succeeds (although it may truncate).  In Cygwin >= 1.7.9, 
> instead
> +   of truncating, it leaves the buffer untouched.  */

More accurately:

The change in strerror_r behavior was in 1.7.8 (which also tried to
introduce __xpg_strerror_r), but a bug in the headers vs. export table
ended up with a link failure for __xpg_strerror_r:
http://cygwin.com/ml/cygwin/2011-03/msg00247.html

In <= 1.7.7, the result was always buf, and truncation occurred for both
in-range and out-of-range errors.

In >= 1.7.8, the result for in-range buffers is static and buf is
untouched, and out-of-range errors returns buf with possible truncation.


> +{
> +  char stackbuf[256];
> +
> +  if (buflen < sizeof (stackbuf))
> +{
> +  size_t len;
> +
> +  stackbuf[0] = '\0'; /* in case strerror_r does nothing */
> +  strerror_r (errnum, stackbuf, sizeof (stackbuf));
> +  len = strlen (stackbuf);
> +  if (len < buflen)
> +{
> +  memcpy (buf, stackbuf, len + 1);
> +  ret = 0;
> +}
> +  else
> +ret = ERANGE;
> +}
> +  else
> +{
> +  buf[0] = '\0'; /* in case strerror_r does nothing */
> +  strerror_r (errnum, buf, buflen);
> +  ret = 0;
> +}
> +}

Yes, that looks good, other than any comment wording tweaks you might want.

[Actually, I plan on further enhancing gnulib strerror_r to work around
http://sourceware.org/bugzilla/show_bug.cgi?id=12782, which may cause me
to have to revisit this code at the same time I add in glibc
workarounds, but that can come after your patch]

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] strerror_r: fix on newer cygwin

[Bruno Haible]

Eric Blake wrote:
> More accurately:
> 
> The change in strerror_r behavior was in 1.7.8 (which also tried to
> introduce __xpg_strerror_r), but a bug in the headers vs. export table
> ended up with a link failure for __xpg_strerror_r:
> http://cygwin.com/ml/cygwin/2011-03/msg00247.html
> 
> In <= 1.7.7, the result was always buf, and truncation occurred for both
> in-range and out-of-range errors.
> 
> In >= 1.7.8, the result for in-range buffers is static and buf is
> untouched, and out-of-range errors returns buf with possible truncation.

Thanks for the info. I've updated the comments accordingly.

> Yes, that looks good, other than any comment wording tweaks you might want.

OK. Committed and pushed.

> [Actually, I plan on further enhancing gnulib strerror_r to work around
> http://sourceware.org/bugzilla/show_bug.cgi?id=12782, which may cause me
> to have to revisit this code at the same time I add in glibc
> workarounds, but that can come after your patch]

I would find these enhancements to the strerror_r specification useful
as well. But I would wait before changing gnulib until the issue has been
discussed in the Austin group and glibc has followed suit. If they start
to disagree, it's too early for gnulib to provide the modified (proposed)
semantics.

Bruno
-- 
In memoriam Anne Boleyn 

Re: proposed new module intprops-test

[Paul Eggert]

On 05/19/11 06:07, Bruno Haible wrote:
> Thanks, this patch fixed the errors on OSF/1 5.1 and reduced the errors on
> Solaris 9 to only one:...

Yes, thanks, I've committed something which I hope fixes that;
see below.

> This part of the patch has no effect on HP-UX, IRIX, and neither on
> OSF/1 and Solaris where the test already compiles fine. You could just as
> well revert it.

OK, done.

> Analysis of the HP-UX cc bug:...
> So I think, you will have to use ASSERT here instead of verify.

Thanks, done.

> Analysis of the IRIX cc bug:
> So here apparently the problem is adding an unsigned value and -1.
> The following change fixes this error for me:
> 
> #define _GL_INT_SIGNED(e) (_GL_INT_CONVERT (e, -1) < 0)
> #undef _GL_INT_SIGNED
> #define _GL_INT_SIGNED(e) ((e) - (e) - 1 < 0)

OK, thanks, I committed something a bit more general.

> Other errors still remain; to be investigated in the next round.

Thanks for all that checking!  Here's what I committed:

>From 55abd3179923a7ec9cde845b8c37fc678c31dd5c Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Thu, 19 May 2011 19:15:19 -0700
Subject: [PATCH 1/3] intprops-tests: revert unsigned part of previous change

* tests/test-intprops.c (UINT_MAX, ULONG_MAX, UINTMAX_MAX, U0, U1):
Remove; they weren't actually needed.  All uses of U0 and U1 removed,
and other casts to 'unsigned int' reverted to 'u' suffixes.  See
.
---
 ChangeLog |8 
 tests/test-intprops.c |  102 +---
 2 files changed, 53 insertions(+), 57 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index e9da004..5065b4b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2011-05-19  Paul Eggert  
+
+   intprops-tests: revert unsigned part of previous change
+   * tests/test-intprops.c (UINT_MAX, ULONG_MAX, UINTMAX_MAX, U0, U1):
+   Remove; they weren't actually needed.  All uses of U0 and U1 removed,
+   and other casts to 'unsigned int' reverted to 'u' suffixes.  See
+   .
+
 2011-05-19  Bruno Haible  
 
strerror_r: Work around strerror_r() change in Cygwin 1.7.8.
diff --git a/tests/test-intprops.c b/tests/test-intprops.c
index 9c2fe07..eec35dd 100644
--- a/tests/test-intprops.c
+++ b/tests/test-intprops.c
@@ -26,18 +26,6 @@
 
 #include "macros.h"
 
-/* Work around compiler bugs in HP-UX 11.23 cc, IRIX 6.5 cc, OSF/1 5.1
-   cc, and Solaris 9 cc.  See
-   .  */
-#undef UINT_MAX
-#undef ULONG_MAX
-#undef UINTMAX_MAX
-#define UINT_MAX ((unsigned int) -1)
-#define ULONG_MAX ((unsigned long int) -1)
-#define UINTMAX_MAX ((uintmax_t) -1)
-#define U0 ((unsigned int) 0)
-#define U1 ((unsigned int) 1)
-
 /* Integer representation.  */
 verify (INT_MIN + INT_MAX < 0
 ? (TYPE_TWOS_COMPLEMENT (int)
@@ -112,23 +100,23 @@ verify (check_binop (ADD, INT_MAX, 1, INT_MIN, INT_MAX, 
true));
 verify (check_binop (ADD, INT_MAX, -1, INT_MIN, INT_MAX, false));
 verify (check_binop (ADD, INT_MIN, 1, INT_MIN, INT_MAX, false));
 verify (check_binop (ADD, INT_MIN, -1, INT_MIN, INT_MAX, true));
-verify (check_binop (ADD, UINT_MAX, U1, U0, UINT_MAX, true));
-verify (check_binop (ADD, U0, U1, U0, UINT_MAX, false));
+verify (check_binop (ADD, UINT_MAX, 1u, 0u, UINT_MAX, true));
+verify (check_binop (ADD, 0u, 1u, 0u, UINT_MAX, false));
 
 verify (check_binop (SUBTRACT, INT_MAX, 1, INT_MIN, INT_MAX, false));
 verify (check_binop (SUBTRACT, INT_MAX, -1, INT_MIN, INT_MAX, true));
 verify (check_binop (SUBTRACT, INT_MIN, 1, INT_MIN, INT_MAX, true));
 verify (check_binop (SUBTRACT, INT_MIN, -1, INT_MIN, INT_MAX, false));
-verify (check_binop (SUBTRACT, UINT_MAX, U1, U0, UINT_MAX, false));
-verify (check_binop (SUBTRACT, U0, U1, U0, UINT_MAX, true));
+verify (check_binop (SUBTRACT, UINT_MAX, 1u, 0u, UINT_MAX, false));
+verify (check_binop (SUBTRACT, 0u, 1u, 0u, UINT_MAX, true));
 
 verify (check_unop (NEGATE, INT_MIN, INT_MIN, INT_MAX,
 TYPE_TWOS_COMPLEMENT (int)));
 verify (check_unop (NEGATE, 0, INT_MIN, INT_MAX, false));
 verify (check_unop (NEGATE, INT_MAX, INT_MIN, INT_MAX, false));
-verify (check_unop (NEGATE, U0, U0, UINT_MAX, false));
-verify (check_unop (NEGATE, U1, U0, UINT_MAX, true));
-verify (check_unop (NEGATE, UINT_MAX, U0, UINT_MAX, true));
+verify (check_unop (NEGATE, 0u, 0u, UINT_MAX, false));
+verify (check_unop (NEGATE, 1u, 0u, UINT_MAX, true));
+verify (check_unop (NEGATE, UINT_MAX, 0u, UINT_MAX, true));
 
 verify (check_binop (MULTIPLY, INT_MAX, INT_MAX, INT_MIN, INT_MAX, true));
 verify (check_binop (MULTIPLY, INT_MAX, INT_MIN, INT_MIN, INT_MAX, true));
@@ -143,17 +131,17 @@ verify (check_binop (DIVIDE, INT_MIN, -1, INT_MIN, 
INT_MAX,
  INT_NEGATE_OVERFLOW (INT_MIN)));
 verify (check_binop (DIVIDE, INT_MAX, 1, INT_MIN, INT_MAX, false));
 verify (check_binop (DIVIDE, (unsi