Re: test-intprops.c: many new warnings

[Jim Meyering]

Paul Eggert wrote:
> On 05/24/11 12:36, Jim Meyering wrote:
>> "make check" was inundated with new warnings.
>> Nearly 500 lines worth.
>
> Thanks, I fixed those by pushing the following two patches:

Those look fine and work for me.
Thanks!

Re: test-intprops.c: many new warnings

[Eric Blake]

On 05/24/2011 05:50 PM, Paul Eggert wrote:
> +++ b/tests/test-intprops.c
> @@ -16,6 +16,12 @@
>  
>  /* Written by Paul Eggert.  */
>  
> +/* Tell gcc not to warn about the many (X < 0) expressions that
> +   the overflow macros expand to.  */
> +#if (__GNUC__ == 4 && 3 <= __GNUC_MINOR__) || 4 < __GNUC__
> +# pragma GCC diagnostic ignored "-Wtype-limits"
> +#endif

Would it be possible instead to write forwarding macros?  That is, the
outer macro calls concat(name,test(params)) where test expands to either
0 or 1, at which point name##result can be used to call name0 for the
unsigned case (no X < 0) or name1 for the signed case (including X < 0)
checks?

Otherwise, we are just disabling the warnings for this test, but the
warnings will still byte us in regular code.  I guess I'll try to find
time to play with the idea instead.

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



signature.asc
Description: OpenPGP digital signature

Re: test-intprops.c: many new warnings

[Paul Eggert]

On 05/24/11 12:36, Jim Meyering wrote:

> "make check" was inundated with new warnings.
> Nearly 500 lines worth.

Thanks, I fixed those by pushing the following two patches:

---
 ChangeLog  |9 +
 lib/intprops.h |8 
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 270866e..fee8b5a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2011-05-24  Paul Eggert  
+
+   intprops: shorten, to pacify gcc -Woverlength-strings
+   * lib/intprops.h (_GL_INT_CONVERT, _GL_INT_NEGATE_CONVERT):
+   (_GL_BINARY_OP_OVERFLOW): Say "0 * (x)" rather than "(x) - (x)",
+   so that, for example, verify (INT_MULTIPLY_OVERFLOW (...)) is less
+   likely to run afoul of C compiler limits for string constant lengths.
+   See 
.
+
 2011-05-24  Eric Blake  
 
docs: document recently fixed glibc printf bug
diff --git a/lib/intprops.h b/lib/intprops.h
index 293204a..d722648 100644
--- a/lib/intprops.h
+++ b/lib/intprops.h
@@ -25,11 +25,11 @@
 /* Return a integer value, converted to the same type as the integer
expression E after integer type promotion.  V is the unconverted value.
E should not have side effects.  */
-#define _GL_INT_CONVERT(e, v) ((e) - (e) + (v))
+#define _GL_INT_CONVERT(e, v) (0 * (e) + (v))
 
 /* Act like _GL_INT_CONVERT (E, -V) but work around a bug in IRIX 6.5 cc; see
.  */
-#define _GL_INT_NEGATE_CONVERT(e, v) ((e) - (e) - (v))
+#define _GL_INT_NEGATE_CONVERT(e, v) (0 * (e) - (v))
 
 /* The extra casts in the following macros work around compiler bugs,
e.g., in Cray C 5.0.3.0.  */
@@ -314,7 +314,7 @@
Arguments should be free of side effects.  */
 #define _GL_BINARY_OP_OVERFLOW(a, b, op_result_overflow)\
   op_result_overflow (a, b, \
-  _GL_INT_MINIMUM ((b) - (b) + (a)),\
-  _GL_INT_MAXIMUM ((b) - (b) + (a)))
+  _GL_INT_MINIMUM (0 * (b) + (a)),  \
+  _GL_INT_MAXIMUM (0 * (b) + (a)))
 
 #endif /* _GL_INTPROPS_H */
-- 
1.7.4.4



---
 ChangeLog |6 ++
 tests/test-intprops.c |6 ++
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index fee8b5a..390d4e8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
 2011-05-24  Paul Eggert  
 
+   test-intprops: disable -Wtype-limits diagnostics
+   * tests/test-intprops.c: Use a pragma to ignore -Wtype-limits
+   diagnostics.  Otherwise, the integer overflow macros generate many
+   diagnostics.  Reported by Jim Meyering in
+   .
+
intprops: shorten, to pacify gcc -Woverlength-strings
* lib/intprops.h (_GL_INT_CONVERT, _GL_INT_NEGATE_CONVERT):
(_GL_BINARY_OP_OVERFLOW): Say "0 * (x)" rather than "(x) - (x)",
diff --git a/tests/test-intprops.c b/tests/test-intprops.c
index 8fc582b..1a34d77 100644
--- a/tests/test-intprops.c
+++ b/tests/test-intprops.c
@@ -16,6 +16,12 @@
 
 /* Written by Paul Eggert.  */
 
+/* Tell gcc not to warn about the many (X < 0) expressions that
+   the overflow macros expand to.  */
+#if (__GNUC__ == 4 && 3 <= __GNUC_MINOR__) || 4 < __GNUC__
+# pragma GCC diagnostic ignored "-Wtype-limits"
+#endif
+
 #include 
 
 #include "intprops.h"
-- 
1.7.4.4

[PATCH] docs: document recently fixed glibc printf bug

[Eric Blake]

Document it as a known bug, but one where we don't provide a
workaround since programmers are unlikely to hit it in practice.

* doc/posix-functions/fprintf.texi (fprintf): Document it.
* doc/posix-functions/printf.texi (printf): Likewise.
* doc/posix-functions/vfprintf.texi (vfprintf): Likewise.
* doc/posix-functions/vprintf.texi (vprintf): Likewise.

Signed-off-by: Eric Blake 
---
 ChangeLog |6 ++
 doc/posix-functions/fprintf.texi  |4 
 doc/posix-functions/printf.texi   |4 
 doc/posix-functions/vfprintf.texi |4 
 doc/posix-functions/vprintf.texi  |4 
 5 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 6ac14e8..270866e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
 2011-05-24  Eric Blake  

+   docs: document recently fixed glibc printf bug
+   * doc/posix-functions/fprintf.texi (fprintf): Document it.
+   * doc/posix-functions/printf.texi (printf): Likewise.
+   * doc/posix-functions/vfprintf.texi (vfprintf): Likewise.
+   * doc/posix-functions/vprintf.texi (vprintf): Likewise.
+
closein-tests: convert to init.sh
* modules/closein-tests (Files): Add init.sh
* tests/test-closein.sh Use it.
diff --git a/doc/posix-functions/fprintf.texi b/doc/posix-functions/fprintf.texi
index 04c4c72..44bcca3 100644
--- a/doc/posix-functions/fprintf.texi
+++ b/doc/posix-functions/fprintf.texi
@@ -83,4 +83,8 @@ fprintf

 Portability problems not fixed by Gnulib:
 @itemize
+@item
+Attempting to write to a read-only stream fails with @code{EOF} but
+does not set the error flag for @code{ferror} on some platforms:
+glibc 2.13.
 @end itemize
diff --git a/doc/posix-functions/printf.texi b/doc/posix-functions/printf.texi
index df7813f..8fc8cb0 100644
--- a/doc/posix-functions/printf.texi
+++ b/doc/posix-functions/printf.texi
@@ -83,4 +83,8 @@ printf

 Portability problems not fixed by Gnulib:
 @itemize
+@item
+Attempting to write to a read-only stream fails with @code{EOF} but
+does not set the error flag for @code{ferror} on some platforms:
+glibc 2.13.
 @end itemize
diff --git a/doc/posix-functions/vfprintf.texi 
b/doc/posix-functions/vfprintf.texi
index b40a334..c6fab25 100644
--- a/doc/posix-functions/vfprintf.texi
+++ b/doc/posix-functions/vfprintf.texi
@@ -83,4 +83,8 @@ vfprintf

 Portability problems not fixed by Gnulib:
 @itemize
+@item
+Attempting to write to a read-only stream fails with @code{EOF} but
+does not set the error flag for @code{ferror} on some platforms:
+glibc 2.13.
 @end itemize
diff --git a/doc/posix-functions/vprintf.texi b/doc/posix-functions/vprintf.texi
index 342d182..21d4bc6 100644
--- a/doc/posix-functions/vprintf.texi
+++ b/doc/posix-functions/vprintf.texi
@@ -83,4 +83,8 @@ vprintf

 Portability problems not fixed by Gnulib:
 @itemize
+@item
+Attempting to write to a read-only stream fails with @code{EOF} but
+does not set the error flag for @code{ferror} on some platforms:
+glibc 2.13.
 @end itemize
-- 
1.7.4.4

[PATCH 2/3] yesno-tests: convert to init.sh

[Eric Blake]

* modules/yesno-tests (Files): Add init.sh.
* tests/test-yesno.sh: Use it.

Signed-off-by: Eric Blake 
---
 ChangeLog   |4 +++
 modules/yesno-tests |1 +
 tests/test-yesno.sh |   56 +-
 3 files changed, 29 insertions(+), 32 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 31b9c15..db85276 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
 2011-05-24  Eric Blake  

+   yesno-tests: convert to init.sh
+   * modules/yesno-tests (Files): Add init.sh.
+   * tests/test-yesno.sh: Use it.
+
atexit-tests: ensure reliable exit status
* tests/test-atexit.sh: Prefer 'Exit' over 'exit'.
Reported by Bruno Haible.
diff --git a/modules/yesno-tests b/modules/yesno-tests
index 01ba8e1..3e1e0a2 100644
--- a/modules/yesno-tests
+++ b/modules/yesno-tests
@@ -1,4 +1,5 @@
 Files:
+tests/init.sh
 tests/test-yesno.c
 tests/test-yesno.sh

diff --git a/tests/test-yesno.sh b/tests/test-yesno.sh
index b1a5b65..5e5ed40 100755
--- a/tests/test-yesno.sh
+++ b/tests/test-yesno.sh
@@ -1,10 +1,6 @@
 #!/bin/sh
-
-tmpfiles=
-trap 'rm -fr $tmpfiles' 1 2 3 15
-
-p=t-yesno-
-tmpfiles="${p}in.tmp ${p}xout.tmp ${p}out1.tmp ${p}out.tmp ${p}err.tmp"
+: ${srcdir=.}
+. "$srcdir/init.sh"; path_prepend_ .

 # For now, only test with C locale
 LC_ALL=C
@@ -19,7 +15,7 @@ else
 fi

 # Test with seekable stdin; the followon process must see remaining data.
-tr @ '\177' < ${p}in.tmp
+tr @ '\177' < in.tmp
 n - entire line consumed
 y@n - backspace does not change result
 y
@@ -27,7 +23,7 @@ does not match either yesexpr or noexpr
 n
 EOF

-cat < ${p}xout.tmp
+cat < xout.tmp
 N
 Y
 Y
@@ -35,40 +31,36 @@ N
 n
 EOF

-(./test-yesno${EXEEXT}; ./test-yesno${EXEEXT} 3; cat) \
-  < ${p}in.tmp > ${p}out1.tmp || exit 1
-LC_ALL=C tr -d "$cr" < ${p}out1.tmp > ${p}out.tmp || exit 1
-cmp ${p}xout.tmp ${p}out.tmp || exit 1
+fail=0
+(test-yesno; test-yesno 3; cat) < in.tmp > out1.tmp || fail=1
+LC_ALL=C tr -d "$cr" < out1.tmp > out.tmp || fail=1
+cmp xout.tmp out.tmp || fail=1

-(./test-yesno${EXEEXT} 3; ./test-yesno${EXEEXT}; cat) \
-  < ${p}in.tmp > ${p}out1.tmp || exit 1
-LC_ALL=C tr -d "$cr" < ${p}out1.tmp > ${p}out.tmp || exit 1
-cmp ${p}xout.tmp ${p}out.tmp || exit 1
+(test-yesno 3; test-yesno; cat) < in.tmp > out1.tmp || fail=1
+LC_ALL=C tr -d "$cr" < out1.tmp > out.tmp || fail=1
+cmp xout.tmp out.tmp || fail=1

 # Test for behavior on pipe
-cat < ${p}xout.tmp
+cat < xout.tmp
 Y
 N
 EOF
-echo yes | ./test-yesno${EXEEXT} 2 > ${p}out1.tmp || exit 1
-LC_ALL=C tr -d "$cr" < ${p}out1.tmp > ${p}out.tmp || exit 1
-cmp ${p}xout.tmp ${p}out.tmp || exit 1
+echo yes | test-yesno 2 > out1.tmp || fail=1
+LC_ALL=C tr -d "$cr" < out1.tmp > out.tmp || fail=1
+cmp xout.tmp out.tmp || fail=1

 # Test for behavior on EOF
-cat < ${p}xout.tmp
+cat < xout.tmp
 N
 EOF
-./test-yesno${EXEEXT}  ${p}out1.tmp || exit 1
-LC_ALL=C tr -d "$cr" < ${p}out1.tmp > ${p}out.tmp || exit 1
-cmp ${p}xout.tmp ${p}out.tmp || exit 1
+test-yesno  out1.tmp || fail=1
+LC_ALL=C tr -d "$cr" < out1.tmp > out.tmp || fail=1
+cmp xout.tmp out.tmp || fail=1

 # Test for behavior when stdin is closed
-./test-yesno${EXEEXT} 0 <&- > ${p}out1.tmp 2> ${p}err.tmp && exit 1
-LC_ALL=C tr -d "$cr" < ${p}out1.tmp > ${p}out.tmp || exit 1
-cmp ${p}xout.tmp ${p}out.tmp || exit 1
-test -s ${p}err.tmp || exit 1
-
-# Cleanup
-rm -fr $tmpfiles
+test-yesno 0 <&- > out1.tmp 2> err.tmp && fail=1
+LC_ALL=C tr -d "$cr" < out1.tmp > out.tmp || fail=1
+cmp xout.tmp out.tmp || fail=1
+test -s err.tmp || fail=1

-exit 0
+Exit $fail
-- 
1.7.4.4

[PATCH 3/3] closein-tests: convert to init.sh

[Eric Blake]

* modules/closein-tests (Files): Add init.sh
* tests/test-closein.sh Use it.

Signed-off-by: Eric Blake 
---
 ChangeLog |4 
 modules/closein-tests |1 +
 tests/test-closein.sh |   38 --
 3 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index db85276..6ac14e8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
 2011-05-24  Eric Blake  

+   closein-tests: convert to init.sh
+   * modules/closein-tests (Files): Add init.sh
+   * tests/test-closein.sh Use it.
+
yesno-tests: convert to init.sh
* modules/yesno-tests (Files): Add init.sh.
* tests/test-yesno.sh: Use it.
diff --git a/modules/closein-tests b/modules/closein-tests
index 2f3f689..8cbf593 100644
--- a/modules/closein-tests
+++ b/modules/closein-tests
@@ -1,4 +1,5 @@
 Files:
+tests/init.sh
 tests/test-closein.sh
 tests/test-closein.c

diff --git a/tests/test-closein.sh b/tests/test-closein.sh
index a75929a..27b7929 100755
--- a/tests/test-closein.sh
+++ b/tests/test-closein.sh
@@ -1,38 +1,32 @@
 #!/bin/sh
+: ${srcdir=.}
+. "$srcdir/init.sh"; path_prepend_ .

-tmpfiles=
-trap 'rm -fr $tmpfiles' 1 2 3 15
-
-p=t-closein-
-tmpfiles="${p}in.tmp ${p}xout.tmp ${p}out1.tmp ${p}out2.tmp"
-
-echo Hello world > ${p}in.tmp
-echo world > ${p}xout.tmp
+echo Hello world > in.tmp
+echo world > xout.tmp

+fail=0
 # Test with seekable stdin; followon process must see remaining data
-(./test-closein${EXEEXT}; cat) < ${p}in.tmp > ${p}out1.tmp || exit 1
-cmp ${p}out1.tmp ${p}in.tmp || exit 1
+(test-closein; cat) < in.tmp > out1.tmp || fail=1
+cmp out1.tmp in.tmp || fail=1

-(./test-closein${EXEEXT} consume; cat) < ${p}in.tmp > ${p}out2.tmp || exit 1
-cmp ${p}out2.tmp ${p}xout.tmp || exit 1
+(test-closein consume; cat) < in.tmp > out2.tmp || fail=1
+cmp out2.tmp xout.tmp || fail=1

 # Test for lack of error on pipe.  Ignore any EPIPE failures from cat.
-cat ${p}in.tmp 2>/dev/null | ./test-closein${EXEEXT} || exit 1
+cat in.tmp 2>/dev/null | test-closein || fail=1

-cat ${p}in.tmp 2>/dev/null | ./test-closein${EXEEXT} consume || exit 1
+cat in.tmp 2>/dev/null | test-closein consume || fail=1

 # Test for lack of error when nothing is read
-./test-closein${EXEEXT} /dev/null && exit 1
-
-# Cleanup
-rm -fr $tmpfiles
+test-closein consume close <&- 2>/dev/null && fail=1

-exit 0
+Exit $fail
-- 
1.7.4.4

[PATCH 1/3] atexit-tests: ensure reliable exit status

[Eric Blake]

This was the only remaining init.sh client that didn't properly
use the 'Exit' function.

* tests/test-atexit.sh: Prefer 'Exit' over 'exit'.
Reported by Bruno Haible.

Signed-off-by: Eric Blake 
---

> Shouldn't this patch also be applied to tests/test-atexit.sh?
> It's the first test that uses init.sh (in alphabetical order), therefore
> people might want to use it as a template.

Yes.  And in fact, it was the only one that wasn't properly using 'Exit'.

While I was in the area, I also converted a couple other tests.

 ChangeLog|6 ++
 tests/test-atexit.sh |2 +-
 2 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 1f0ac8b..31b9c15 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2011-05-24  Eric Blake  
+
+   atexit-tests: ensure reliable exit status
+   * tests/test-atexit.sh: Prefer 'Exit' over 'exit'.
+   Reported by Bruno Haible.
+
 2011-05-24  Bruno Haible  

strerror_r-posix: Respect rules for use of AC_LIBOBJ.
diff --git a/tests/test-atexit.sh b/tests/test-atexit.sh
index 05f23eb..643a72f 100755
--- a/tests/test-atexit.sh
+++ b/tests/test-atexit.sh
@@ -25,4 +25,4 @@ if test -f t-atexit.tmp; then
   Exit 1
 fi

-exit 0
+Exit 0
-- 
1.7.4.4

Re: fprintf failures on read-only streams

[Bruno Haible]

Hi Eric,

> Just last week, glibc fixed a bug where vfprintf failed to set ferror()
> when returning EOF due to an attempt to output to a read-only stream:
> 
> http://sourceware.org/git/?p=glibc.git;a=commitdiff;h=aec84f5395
> 
> Should we update fprintf-posix and friends to detect this bug and work
> around it on older glibc?

I would say no. Hardly any program could be affected by this pitfall,
because whether a stream is opened as read-write or read-only is part of
the logic of the program, and if a programmer does it wrong, he will
notice it quickly on any platform.

> And at any rate, we probably need to update 
> docs/posix-functions/[v][f]printf to document it.

Yes, of course.

Bruno
-- 
In memoriam Georges Darboy 

Re: request: pointer to the docs in the module file

[Bruno Haible]

Sam Steingold wrote:
> >> How do I find out where a module is documented?

Look at the gnulib manual TOC
  
If it's not a ISO C / POSIX or GNU substitute module, then take a look
at the chapter "Particular Modules".

> I usually cannot tell right away
> whether a specific function is posix or glibc (e.g., I thought that
> mkdtemp was not in posix)

Either search in the above TOC. Or use the reference to POSIX
  

Or do a "man ..." - the manual page often says to which standards a function
complies.

> Suppose I want to replace my very own maze of #ifdefs surrounding a call
> to a posix function foo() with a nice gnulib module foo.
> Right now I look for modules/foo

This is a good heuristic, because we try to be consistent in the naming of
modules. But there are a few inconsistent cases. Therefore it is more
reliable to go by the documentation's TOC.

> So, if I were using the mkdir module ..., I can use mkdir with
> 2 arguments on mingw too?

Yes.

> But which header file will defined mkdir with 2 arguments?

That's the 'Include' field of the module description. In modules/mkdir it reads:

  Include:
  

> The mkdir module does not seem to export any header files.

It does not explicitly bring in any header file. But it gives you the guarantee
that you can use mkdir() with 2 arguments after including .

> Oh wait - it will replace mkdir with a _function_ rpl_mkdir, right?

Function or inline function or macro - that is a detail. Recently we have
switched many things from macros to functions, because that allows gnulib
to be reasonably used in C++ programs.

> > That is, docs/posix-functions is intended to be a catch-all for _all_
> > portability problems, not just those fixed by gnulib, but tends to be
> > biased towards gnulib solutions.
> 
> I am sorry, I can parse this sentence just fine, but I cannot extract
> the meaning.  Remember, English is not my native language.

He means that in any doc/posix-functions/*.texi file we list all portability
problems that we know of - both fixed by gnulib and those we've considered
low priority -. But we certainly miss some problems that we've never
encountered.

Bruno
-- 
In memoriam Georges Darboy 

Re: [PATCH 3/3] perror: work around FreeBSD bug

[Bruno Haible]

Eric Blake wrote:
> I'm still seeing perror failures on at least
> AIX and Irix, due to strerror_r but not perror being replaced.  I'm not
> quite sure of the right m4 fix to make, but hopefully I can fix this
> soon (first, though, I plan on completing my strerror_r work).

I'm applying one of the patches from the "better use of AC_LIBOBJ" patch
series. Now, you can change gl_FUNC_PERROR to require gl_FUNC_STRERROR_R
and then test the values of HAVE_DECL_STRERROR_R and REPLACE_STRERROR_R.


2011-05-24  Bruno Haible  

strerror_r-posix: Respect rules for use of AC_LIBOBJ.
* m4/strerror_r.m4 (gl_FUNC_STRERROR_R): Move AC_LIBOBJ and
gl_PREREQ_STRERROR_R invocations from here...
* modules/strerror_r-posix (configure.ac): ... to here.

diff --git a/m4/strerror_r.m4 b/m4/strerror_r.m4
index ddaf10f..1c0af36 100644
--- a/m4/strerror_r.m4
+++ b/m4/strerror_r.m4
@@ -1,4 +1,4 @@
-# strerror_r.m4 serial 8
+# strerror_r.m4 serial 9
 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,
@@ -100,10 +100,6 @@ changequote([,])dnl
   REPLACE_STRERROR_R=1
 fi
   fi
-  if test $HAVE_DECL_STRERROR_R = 0 || test $REPLACE_STRERROR_R = 1; then
-AC_LIBOBJ([strerror_r])
-gl_PREREQ_STRERROR_R
-  fi
 ])
 
 # Prerequisites of lib/strerror_r.c.
diff --git a/modules/strerror_r-posix b/modules/strerror_r-posix
index a36f2e6..90c9806 100644
--- a/modules/strerror_r-posix
+++ b/modules/strerror_r-posix
@@ -13,6 +13,10 @@ lock[test $HAVE_DECL_STRERROR_R = 0 || test 
$REPLACE_STRERROR_R = 1]
 
 configure.ac:
 gl_FUNC_STRERROR_R
+if test $HAVE_DECL_STRERROR_R = 0 || test $REPLACE_STRERROR_R = 1; then
+  AC_LIBOBJ([strerror_r])
+  gl_PREREQ_STRERROR_R
+fi
 gl_STRING_MODULE_INDICATOR([strerror_r])
 
 Makefile.am:

-- 
In memoriam Georges Darboy 

Re: [PATCH 2/2] perror: avoid spurious test failure on HP-UX

[Bruno Haible]

Eric Blake wrote:
> The next-to-last command in this test has non-zero status.  Even
> though 'exit 0' is supposed to ignore prior status, HP-UX /bin/sh
> favors the prior status if an exit trap is installed.
> 
> * tests/test-perror.sh: Use Exit to avoid wrong exit status.

Shouldn't this patch also be applied to tests/test-atexit.sh?
It's the first test that uses init.sh (in alphabetical order), therefore
people might want to use it as a template.

Bruno
-- 
In memoriam Georges Darboy 

Re: [PATCH] getopt: for ambiguous options, enumerate the possibilities.

[Bruno Haible]

Hi James,

Thanks for having dealt with the second and third point of
.

What about the first one? Can you just add the reference to
 in the ChangeLog entry?
And also, when you talk about "glibc change", add either an URL or the date
of that change? It would help future analyzes a lot to have the precise
references.

> +  if (ambig_list != NULL)
> + {
> +   do
> + {
> +   struct option_list *pn = ambig_list->next;
> +   free (ambig_list);
> +   ambig_list = pn;
> + } while (ambig_list != NULL);
> + }
> +  

It seems to me that I see again tabs here.

Also, this loop could be written in a simpler way as

while (ambig_list != NULL)
  {
struct option_list *pn = ambig_list->next;
free (ambig_list);
ambig_list = pn;
  }

Bruno
-- 
In memoriam Georges Darboy 

Re: getloadavg is broken

[Eric Blake]

On 05/24/2011 03:57 PM, Bruno Haible wrote:
> Sam Steingold wrote:
>> Unless I remove the offending line (see the appended patch), configure
>> fails with "../src/src/gllib/getloadavg.c is missing" message.
>> apparently $srcdir is "../src" and $1=$gl_source_base="src/gllib" which
>> cannot be combined into anything sensible.
> 
> This code was meant to simulate what an AC_LIBSOURCES invocation does.
> But AC_LIBSOURCES from autoconf was found to be inadequate in the context
> of gnulib and therefore has gotten a gnulib specific override (see
> gnulib-tool functions func_emit_initmacro_start, func_emit_initmacro_end).
> 
> Most of the gnulib users actually don't even need the AC_LIBSOURCES invocation
> normally, because gnulib-tool and a correct module description already
> guarantee that source files will be where they are expected.
> 
>> no other module does such a check, so it is not clear why getloadavg should.
> 
> Good point. Here's a proposed patch.

Looks sane to me.

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



signature.asc
Description: OpenPGP digital signature

Re: getloadavg is broken

[Bruno Haible]

Sam Steingold wrote:
> Unless I remove the offending line (see the appended patch), configure
> fails with "../src/src/gllib/getloadavg.c is missing" message.
> apparently $srcdir is "../src" and $1=$gl_source_base="src/gllib" which
> cannot be combined into anything sensible.

This code was meant to simulate what an AC_LIBSOURCES invocation does.
But AC_LIBSOURCES from autoconf was found to be inadequate in the context
of gnulib and therefore has gotten a gnulib specific override (see
gnulib-tool functions func_emit_initmacro_start, func_emit_initmacro_end).

Most of the gnulib users actually don't even need the AC_LIBSOURCES invocation
normally, because gnulib-tool and a correct module description already
guarantee that source files will be where they are expected.

> no other module does such a check, so it is not clear why getloadavg should.

Good point. Here's a proposed patch.


2011-05-24  Bruno Haible  

getloadavg: Remove an unreliable safety check.
* m4/getloadavg.m4 (gl_GETLOADAVG): Drop argument. Remove test
whether getloadavg.c is in place.
* modules/getloadavg (configure.ac): Drop argument of gl_GETLOADAVG.
Reported by Sam Steingold .

--- m4/getloadavg.m4.orig   Tue May 24 23:49:07 2011
+++ m4/getloadavg.m4Tue May 24 23:48:41 2011
@@ -7,23 +7,19 @@
 # gives unlimited permission to copy and/or distribute it,
 # with or without modifications, as long as this notice is preserved.
 
-#serial 2
+#serial 3
 
 # Autoconf defines AC_FUNC_GETLOADAVG, but that is obsolescent.
 # New applications should use gl_GETLOADAVG instead.
 
-# gl_GETLOADAVG(LIBOBJDIR)
-# 
+# gl_GETLOADAVG
+# -
 AC_DEFUN([gl_GETLOADAVG],
 [AC_REQUIRE([gl_STDLIB_H_DEFAULTS])
 
 # Persuade glibc  to declare getloadavg().
 AC_REQUIRE([AC_USE_SYSTEM_EXTENSIONS])
 
-# Make sure getloadavg.c is where it belongs, at configure-time.
-test -f "$srcdir/$1/getloadavg.c" ||
-  AC_MSG_ERROR([$srcdir/$1/getloadavg.c is missing])
-
 gl_save_LIBS=$LIBS
 
 # getloadvg is present in libc on glibc >= 2.2, MacOS X, FreeBSD >= 2.0,
--- modules/getloadavg.orig Tue May 24 23:49:07 2011
+++ modules/getloadavg  Tue May 24 23:48:51 2011
@@ -12,7 +12,7 @@
 stdlib
 
 configure.ac:
-gl_GETLOADAVG([$gl_source_base])
+gl_GETLOADAVG
 gl_STDLIB_MODULE_INDICATOR([getloadavg])
 
 Makefile.am:
-- 
In memoriam Georges Darboy 

Re: strerror vs. threads

[Eric Blake]

On 05/24/2011 03:30 PM, Simon Josefsson wrote:
> Maybe there could be a strerror_r-gnu and strerror_r-posix choice?  Or a
> strerror_r-posixlean to just do the minimum required by POSIX.

I think a strerror_r-gnu module might be nice, especially now that I've
posted a patch series that decouples strerror and strerror_r-posix.
Unfortunately, though, we need to think about what happens if both
strerror_r-gnu and strerror_r-posix are imported into the same project
(perhaps because _you_ want to use the GNU signature in your code, but
you also want to use the perror module which implemented using the POSIX
signature)?  Which signature wins in each file, and how do you ensure
that all code is compiled with the signature it is expecting?
Therefore, I'm afraid that the more portable choice is to consistently
stick to the standard definition, rather than the GNU definition, even
if the GNU definition is slightly easier to use.

> Further, having a strerror_r that returns the same string for two
> different but equally bogus inputs is harmless to me -- even IF the code
> path is triggered in the real world, and IF there actually is a problem,
> the user won't be happier to see "Unknown error 4711" instead of
> "Unknown error".  A developer is needed to resolve the problem anyway,
> and they can add 'printf ("foo %d\n", errno);' if needed.

The problem is not necessarily with "Unknown error" (which Solaris
uses), but with implementations that return "", or worse leave buf
uninitialized on error (in fact, even glibc 2.13 is guilty of this,
since it was fixed upstream just last weekend).  Since the whole point
of strerror_r is to get an error message to be worth printing, it's
easier to stick with the POSIX recommendations that the message be
usable regardless of error, even though POSIX (for backwards
compatibility reasons) chose to permit weaker implementations.

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



signature.asc
Description: OpenPGP digital signature

[PATCH] getopt: for ambiguous options, enumerate the possibilities.

[James Youngman]

* lib/getopt.c (_getopt_internal_r): Merge glibc change printing
the ambiguous options when an ambiguous prefix is given. This was
glibc Buganizer wishlist bug 7101.
---
 ChangeLog|7 
 lib/getopt.c |   87 ++
 2 files changed, 76 insertions(+), 18 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index c2a67da..02809c5 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2011-05-15  James Youngman  
+
+   getopt: for ambiguous options, enumerate the possibilities.
+   * lib/getopt.c (_getopt_internal_r): Merge glibc change printing
+   the ambiguous options when an ambiguous prefix is given. This was
+   glibc Buganizer wishlist bug 7101.
+
 2011-05-13  Paul Eggert  
 
intprops-tests: new module
diff --git a/lib/getopt.c b/lib/getopt.c
index c8b3013..f0d1cf6 100644
--- a/lib/getopt.c
+++ b/lib/getopt.c
@@ -479,8 +479,14 @@ _getopt_internal_r (int argc, char **argv, const char 
*optstring,
 || !strchr (optstring, argv[d->optind][1])
 {
   char *nameend;
+  unsigned int namelen;
   const struct option *p;
   const struct option *pfound = NULL;
+  struct option_list
+  {
+const struct option *p;
+struct option_list *next;
+  } *ambig_list = NULL;
   int exact = 0;
   int ambig = 0;
   int indfound = -1;
@@ -488,14 +494,14 @@ _getopt_internal_r (int argc, char **argv, const char 
*optstring,
 
   for (nameend = d->__nextchar; *nameend && *nameend != '='; nameend++)
 /* Do nothing.  */ ;
+  namelen = nameend - d->__nextchar;
 
   /* Test all long options for either exact match
  or abbreviated matches.  */
   for (p = longopts, option_index = 0; p->name; p++, option_index++)
-if (!strncmp (p->name, d->__nextchar, nameend - d->__nextchar))
+if (!strncmp (p->name, d->__nextchar, namelen))
   {
-if ((unsigned int) (nameend - d->__nextchar)
-== (unsigned int) strlen (p->name))
+if (namelen == (unsigned int) strlen (p->name))
   {
 /* Exact match found.  */
 pfound = p;
@@ -513,35 +519,71 @@ _getopt_internal_r (int argc, char **argv, const char 
*optstring,
  || pfound->has_arg != p->has_arg
  || pfound->flag != p->flag
  || pfound->val != p->val)
-  /* Second or later nonexact match found.  */
-  ambig = 1;
+  {
+/* Second or later nonexact match found.  */
+struct option_list *newp = malloc (sizeof (*newp));
+newp->p = p;
+newp->next = ambig_list;
+ambig_list = newp;
+  }
   }
 
-  if (ambig && !exact)
+  if (ambig_list != NULL && !exact)
 {
   if (print_errors)
 {
+  struct option_list first;
+  first.p = pfound;
+  first.next = ambig_list;
+  ambig_list = &first;
+
 #if defined _LIBC && defined USE_IN_LIBIO
-  char *buf;
+  char *buf = NULL;
+  size_t buflen = 0;
 
-  if (__asprintf (&buf, _("%s: option '%s' is ambiguous\n"),
-  argv[0], argv[d->optind]) >= 0)
+  FILE *fp = open_memstream (&buf, &buflen);
+  if (fp != NULL)
 {
-  _IO_flockfile (stderr);
+  fprintf (fp,
+   _("%s: option '%s' is ambiguous; possibilities:"),
+   argv[0], argv[d->optind]);
 
-  int old_flags2 = ((_IO_FILE *) stderr)->_flags2;
-  ((_IO_FILE *) stderr)->_flags2 |= _IO_FLAGS2_NOTCANCEL;
+  do
+{
+  fprintf (fp, " '--%s'", ambig_list->p->name);
+  ambig_list = ambig_list->next;
+}
+  while (ambig_list != NULL);
 
-  __fxprintf (NULL, "%s", buf);
+  fputc_unlocked ('\n', fp);
 
-  ((_IO_FILE *) stderr)->_flags2 = old_flags2;
-  _IO_funlockfile (stderr);
+  if (__builtin_expect (fclose (fp) != EOF, 1))
+{
+  _IO_flockfile (stderr);
 
-  free (buf);
+  int old_flags2 = ((_IO_FILE *) stderr)->_flags2;
+  ((_IO_FILE *) stderr)->_flags2 |= _IO_FLAGS2_NOTCANCEL;
+
+  __fxprintf (NULL, "%s", buf);
+
+  ((_IO_FILE *) stderr)->_flags2 = old_flags2;
+  _IO_funlockfile (stderr);
+
+  free (buf);
+}
 }
 #else
-  fprintf (stderr, _("%s: option '%s' is ambiguous\n"),
+  fprintf (stderr,
+   _(

Re: new files imported without new modules added

[Eric Blake]

On 05/24/2011 03:39 PM, Bruno Haible wrote:
>   1) In code that ends up in lisp.run (e.g. errunix.d). This executable has
>  the option to use multiple threads. Therefore strerror_r should be
>  used instead of strerror(), because - as Eric explained - when you
>  call strerror(EACCES) in one thread and strerror(ENOENT) in another
>  thread at nearly the same time, by the time you retrieve the error 
> message
>  in the first thread, it might read "Permissionle or directory".
>  YES, even for fixed error messages, strerror() uses a static buffer
>  in the majority of OSes (at least on NetBSD, HP-UX, native Win32, 
> Cygwin).

Not cygwin (it uses a thread-local buffer for "Unknown error", and
direct string pointers for known errors), but I can indeed confirm that
FreeBSD 8.2 returns the same pointer for multiple strerror() results,
and if that pointer is not thread-local, you have problems in a
multi-threaded app.

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



signature.asc
Description: OpenPGP digital signature

[PATCH 2/2] strerror: drop strerror_r dependency

[Eric Blake]

* modules/strerror-override: New module.
* lib/strerror_r.c (strerror_r): Move gnulib replacement strings...
* lib/strerror-override.c (strerror_override): ...to new file.
* lib/strerror-override.h: Add prototype.
* lib/strerror-impl.h: Delete.
* lib/strerror.c (strerror): New implementation.
* modules/strerror_r-posix (Files): Add new files.
* modules/strerror (Files): Likewise.
(Depends-on): Drop strerror_r-posix.
* MODULES.html.sh: Document new module and strerror_r-posix.
Requested by Sam Steingold.

Signed-off-by: Eric Blake 
---

A new module is required because both strerror and strerror_r might
need strerror-override.o, but they might be in different directories
(one in lib and one in tests), and we can only AC_LIBOBJ it once.

Now, just the strerror module in isolation does not drag in strerror_r.

Tested on glibc with:

gl_cv_header_errno_h_complete=no gl_cv_func_working_strerror=no \
  gl_cv_func_strerror_r_works=no ./gnulib-tool --with-tests \
  --test strerror strerror_r-posix

Hmm, perhaps I should move strerror-override.c to be part of the
errno module, rather than making it a separate module.  That is,
the only time the strerror_override() function should be compiled
is if our replacement errno.h added new values.

 ChangeLog |   13 ++
 MODULES.html.sh   |2 +
 lib/strerror-impl.h   |   42 --
 lib/strerror-override.c   |  347 +
 lib/strerror-override.h   |   48 ++
 lib/strerror.c|   36 +-
 lib/strerror_r.c  |  318 +
 modules/strerror  |9 +-
 modules/strerror-override |   26 
 modules/strerror_r-posix  |1 +
 10 files changed, 479 insertions(+), 363 deletions(-)
 delete mode 100644 lib/strerror-impl.h
 create mode 100644 lib/strerror-override.c
 create mode 100644 lib/strerror-override.h
 create mode 100644 modules/strerror-override

diff --git a/ChangeLog b/ChangeLog
index a66e44d..1aeaa33 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,18 @@
 2011-05-24  Eric Blake  

+   strerror: drop strerror_r dependency
+   * modules/strerror-override: New module.
+   * lib/strerror_r.c (strerror_r): Move gnulib replacement strings...
+   * lib/strerror-override.c (strerror_override): ...to new file.
+   * lib/strerror-override.h: Add prototype.
+   * lib/strerror-impl.h: Delete.
+   * lib/strerror.c (strerror): New implementation.
+   * modules/strerror_r-posix (Files): Add new files.
+   * modules/strerror (Files): Likewise.
+   (Depends-on): Drop strerror_r-posix.
+   * MODULES.html.sh: Document new module and strerror_r-posix.
+   Requested by Sam Steingold.
+
perror: call strerror_r directly
* modules/perror (Files): Drop strerror-impl.h.
* lib/perror.c (perror): Use our own stack buffer, rather than
diff --git a/MODULES.html.sh b/MODULES.html.sh
index 18a7472..26c3fa9 100755
--- a/MODULES.html.sh
+++ b/MODULES.html.sh
@@ -1626,6 +1626,7 @@ func_all_modules ()
   func_module atexit
   func_module strtod
   func_module strerror
+  func_module strerror-override
   func_module mktime
   func_end_table

@@ -1789,6 +1790,7 @@ func_all_modules ()
   func_module strcasestr-simple
   func_module strchrnul
   func_module streq
+  func_module strerror_r-posix
   func_module strnlen
   func_module strnlen1
   func_module strndup
diff --git a/lib/strerror-impl.h b/lib/strerror-impl.h
deleted file mode 100644
index a204243..000
--- a/lib/strerror-impl.h
+++ /dev/null
@@ -1,42 +0,0 @@
-/* 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);
-errno = ret;
-return buf;
-  }
-}
diff --git a/lib/strerror-override.c b/lib/strerror-override.c
new file mode 100644
index 000..5fcbe1f
--- /dev/null
+++ b/lib/strerror-overrid

[PATCH 1/2] perror: call strerror_r directly

[Eric Blake]

No need to make a wrapper that burns static storage when we can
just use stack storage.

* modules/perror (Files): Drop strerror-impl.h.
* lib/perror.c (perror): Use our own stack buffer, rather than
calling a wrapper that uses static storage.
* doc/posix-functions/perror.texi (perror): Document a limitation
of our replacement.

Signed-off-by: Eric Blake 
---

> So about the only other thing we can do is make both strerror.c and
> strerror_r.c call into a third file that implements the gnulib
> replacement strings, and thus make it so that strerror no longer
> directly depends on strerror_r.c; patches welcome.

Thoughts on this series before I push it?

 ChangeLog   |7 +++
 doc/posix-functions/perror.texi |4 
 lib/perror.c|   14 +++---
 modules/perror  |1 -
 4 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 3dc7091..a66e44d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
 2011-05-24  Eric Blake  

+   perror: call strerror_r directly
+   * modules/perror (Files): Drop strerror-impl.h.
+   * lib/perror.c (perror): Use our own stack buffer, rather than
+   calling a wrapper that uses static storage.
+   * doc/posix-functions/perror.texi (perror): Document a limitation
+   of our replacement.
+
strerror_r: fix missing header
* lib/strerror_r.c: Avoid compiler warning about snprintf.

diff --git a/doc/posix-functions/perror.texi b/doc/posix-functions/perror.texi
index cf6ac79..167cf39 100644
--- a/doc/posix-functions/perror.texi
+++ b/doc/posix-functions/perror.texi
@@ -24,4 +24,8 @@ perror
 POSIX requires that this function set the stream error bit (detected
 by @code{ferror}) on write failure, but not all platforms do this:
 glibc 2.13.
+@item
+POSIX requires that this function not alter stream orientation, but
+the gnulib replacement locks in byte orientation and fails on wide
+character streams.
 @end itemize
diff --git a/lib/perror.c b/lib/perror.c
index 29af3c5..88981d1 100644
--- a/lib/perror.c
+++ b/lib/perror.c
@@ -40,10 +40,18 @@
 void
 perror (const char *string)
 {
-  const char *errno_description = my_strerror (errno);
+  char stackbuf[256];
+  int ret;
+
+  /* Our implementation guarantees that this will be a non-empty
+ string, even if it returns EINVAL; and stackbuf should be sized
+ large enough to avoid ERANGE.  */
+  ret = strerror_r (errno, stackbuf, sizeof stackbuf);
+  if (ret == ERANGE)
+abort ();

   if (string != NULL && *string != '\0')
-fprintf (stderr, "%s: %s\n", string, errno_description);
+fprintf (stderr, "%s: %s\n", string, stackbuf);
   else
-fprintf (stderr, "%s\n", errno_description);
+fprintf (stderr, "%s\n", stackbuf);
 }
diff --git a/modules/perror b/modules/perror
index 1ff9ec2..38e28ce 100644
--- a/modules/perror
+++ b/modules/perror
@@ -3,7 +3,6 @@ perror() function: print a message describing error code.

 Files:
 lib/perror.c
-lib/strerror-impl.h
 m4/perror.m4

 Depends-on:
-- 
1.7.4.4

Re: new files imported without new modules added

[Bruno Haible]

Sam Steingold wrote:
> > strerror and perror now depend on strerror_r ...
>
> I do _not_ want strerror_r.

Actually you do want strerror_r in clisp, not strerror.

There are two kinds of uses of strerror() in clisp:

  1) In code that ends up in lisp.run (e.g. errunix.d). This executable has
 the option to use multiple threads. Therefore strerror_r should be
 used instead of strerror(), because - as Eric explained - when you
 call strerror(EACCES) in one thread and strerror(ENOENT) in another
 thread at nearly the same time, by the time you retrieve the error message
 in the first thread, it might read "Permissionle or directory".
 YES, even for fixed error messages, strerror() uses a static buffer
 in the majority of OSes (at least on NetBSD, HP-UX, native Win32, Cygwin).

  2) In the launcher program _clisp.c, the use of strerror() is misplaced:

  if (!CloseHandle(pinfo.hProcess)) goto w32err;
  if (com_initialized) CoUninitialize();
  return exitcode;
 w32err:
  fprintf(stderr,"%s:\n * %s\n * %s\n * [%s]\n = %s\n",program_name,
  executable,resolved,command_line,strerror(GetLastError()));

 On native Windows, there are two distinct enumerations of error codes:
 - the Win32 one, to be retrieved via GetLastError() and to be formatted
   via FormatMessage.
 - the ANSI C one, to be retrieved via 'errno' and to be formatted
   via 'strerror' (which makes a lookup in _sys_errlist, *not* a call
   to FormatMessage).

 So you really need to use FormatMessage here, not strerror().

Bruno
-- 
In memoriam Georges Darboy 

Re: strerror vs. threads

[Simon Josefsson]

Eric Blake  writes:

> On 05/24/2011 12:52 PM, Simon Josefsson wrote:
>>> POSIX explicitly allows strerror to use a static buffer, and that's
>>> _exactly_ what gnulib's implementation does on out-of-range input.
>>> Which means that "Unknown error (-1)" of thread 1 and "Unknown error
>>> (-2)" of thread 2 are calling sprintf on the same memory at the same
>>> time, and you will get indeterminate results.
>> 
>> Which begs the question why the error messages needs to be different for
>> different unknown errors?  Is that required by POSIX?
>
> Not required, but heavily recommended, and guaranteed by several common
> platforms.  And since "Error: " is much more confusing than "Error:
> Unknown error -1", especially at tracking down a bug at why errno is set
> to some weird value, it was worth documenting that we guarantee that
> behavior as one of the points of the gnulib strerror replacement.
>
> If you don't ever pass out-of-range errno values to strerror, then you
> probably don't care about thread-safety of the out-of-range buffer, but
> there is still the question of whether all existing implementations are
> thread-safe even on in-range errno values.

Maybe there could be a strerror_r-gnu and strerror_r-posix choice?  Or a
strerror_r-posixlean to just do the minimum required by POSIX.

For several of my projects that are libraries, any kind of thread
related code is bound to create portability problems.  (I haven't looked
in detail at the thread code here though, so I do not know how well it
handles systems without threads or how well it guesses the system thread
package.)

Further, having a strerror_r that returns the same string for two
different but equally bogus inputs is harmless to me -- even IF the code
path is triggered in the real world, and IF there actually is a problem,
the user won't be happier to see "Unknown error 4711" instead of
"Unknown error".  A developer is needed to resolve the problem anyway,
and they can add 'printf ("foo %d\n", errno);' if needed.

/Simon

getloadavg is broken

[Sam Steingold]

Unless I remove the offending line (see the appended patch), configure
fails with "../src/src/gllib/getloadavg.c is missing" message.
apparently $srcdir is "../src" and $1=$gl_source_base="src/gllib" which
cannot be combined into anything sensible.
no other module does such a check, so it is not clear why getloadavg should.

-- 
Sam Steingold (http://sds.podval.org/) on CentOS release 5.6 (Final) X 
11.0.60900031
http://camera.org http://www.PetitionOnline.com/tap12009/
http://memri.org http://palestinefacts.org http://truepeace.org
Trespassers will be shot.  Survivors will be SHOT AGAIN!


diff -r efa3b7c4c766 src/glm4/getloadavg.m4
--- a/src/glm4/getloadavg.m4Tue May 24 16:34:36 2011 -0400
+++ b/src/glm4/getloadavg.m4Tue May 24 16:57:30 2011 -0400
@@ -20,10 +20,6 @@ AC_DEFUN([gl_GETLOADAVG],
 # Persuade glibc  to declare getloadavg().
 AC_REQUIRE([AC_USE_SYSTEM_EXTENSIONS])
 
-# Make sure getloadavg.c is where it belongs, at configure-time.
-test -f "$srcdir/$1/getloadavg.c" ||
-  AC_MSG_ERROR([$srcdir/$1/getloadavg.c is missing])
-
 gl_save_LIBS=$LIBS
 
 # getloadvg is present in libc on glibc >= 2.2, MacOS X, FreeBSD >= 2.0,

[PATCH] strerror_r: fix missing header

[Eric Blake]

snprintf is not guaranteed to work without a declaration.

* lib/strerror_r.c: Avoid compiler warning about snprintf.

Signed-off-by: Eric Blake 
---
 ChangeLog|3 +++
 lib/strerror_r.c |1 +
 2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 2f31f7d..3dc7091 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,8 @@
 2011-05-24  Eric Blake  

+   strerror_r: fix missing header
+   * lib/strerror_r.c: Avoid compiler warning about snprintf.
+
strerror_r: fix AIX test failures
* lib/strerror_r.c (strerror_r): Convert silent truncation to
ERANGE failure.
diff --git a/lib/strerror_r.c b/lib/strerror_r.c
index 034c22e..494b1f0 100644
--- a/lib/strerror_r.c
+++ b/lib/strerror_r.c
@@ -26,6 +26,7 @@
 #include 

 #include 
+#include 

 #if GNULIB_defined_ESOCK /* native Windows platforms */
 # if HAVE_WINSOCK2_H
-- 
1.7.4.4

Re: disabling multithreading

[Bruno Haible]

Eric Blake wrote:
> for a single-threaded application, having strerror_r call strerror without
> locking is just fine, so the mere use of just the 'strerror' module
> should not drag in locking.  strerror_r only needs locking if it is used
> in a multi-threaded application.

The first step in this direction would be to use change the default choice
of the locking to "no" in the application's configure.ac, as indicated in
  

> That means that strerror-impl.h, strerror_r.c, and strerror_r.m4 will
> still be needed, but it would get rid of the three glthread files.

If people are not satisfied with having the multithread support turned
off by default, and want it turned off always, then we should consider
adding an option --force-disable-threads to gnulib-tool. I haven't yet
thought about how this option makes its effects on the module descriptions.

> I would really like to make strerror_r's use of locking
> conditional on whether 'lock' is independently present.

I understand the wish, but
  - By default, gnulib should be as multithreaded as possible.
Why? Because guaranteeing singlethreaded-ness is something the
gnulib-tool user needs to do explicitly.
  - I would find it harder to achieve the desired behaviour with --avoid=lock
than with a new specific option --force-disable-threads.

Bruno
-- 
In memoriam Georges Darboy 

[PATCH 3/3] strerror_r: fix AIX test failures

[Eric Blake]

Already documented as an AIX limitation.

* lib/strerror_r.c (strerror_r): Convert silent truncation to
ERANGE failure.

Signed-off-by: Eric Blake 
---
 ChangeLog|4 
 lib/strerror_r.c |   20 +++-
 2 files changed, 23 insertions(+), 1 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 80e9369..2f31f7d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
 2011-05-24  Eric Blake  

+   strerror_r: fix AIX test failures
+   * lib/strerror_r.c (strerror_r): Convert silent truncation to
+   ERANGE failure.
+
strerror_r: fix Solaris test failures
* lib/strerror_r.c (strerror_r): Partially populate buf on ERANGE
failures.
diff --git a/lib/strerror_r.c b/lib/strerror_r.c
index f6ce8a3..034c22e 100644
--- a/lib/strerror_r.c
+++ b/lib/strerror_r.c
@@ -473,7 +473,8 @@ strerror_r (int errnum, char *buf, size_t buflen)
   buflen = INT_MAX;

 # ifdef __hpux
-/* On HP-UX 11.31, strerror_r always fails when buflen < 80.  */
+/* On HP-UX 11.31, strerror_r always fails when buflen < 80; it
+   also fails to change buf on EINVAL.  */
 {
   char stackbuf[80];

@@ -501,6 +502,23 @@ strerror_r (int errnum, char *buf, size_t buflen)
   }
 # endif

+# ifdef _AIX
+/* AIX returns 0 rather than ERANGE when truncating strings; try
+   again until we are sure we got the entire string.  */
+if (!ret && strlen (buf) == buflen - 1)
+  {
+char stackbuf[STACKBUF_LEN];
+size_t len;
+strerror_r (errnum, stackbuf, sizeof stackbuf);
+len = strlen (stackbuf);
+/* stackbuf should have been large enough.  */
+if (len + 1 == sizeof stackbuf)
+  abort ();
+if (buflen <= len)
+  ret = ERANGE;
+  }
+# endif
+
 /* Some old implementations may return (-1, EINVAL) instead of EINVAL.  */
 if (ret < 0)
   ret = errno;
-- 
1.7.4.4

[PATCH 1/3] strerror_r: enforce POSIX recommendations

[Eric Blake]

POSIX recommends (but does not require) that strerror_r populate
buf even on error.  But since we guarantee this behavior for
strerror, we might as well also guarantee it for strerror_r.

* lib/strerror_r.c (safe_copy): New helper method.
(strerror_r): Guarantee a non-empty string.
* tests/test-strerror_r.c (main): Enhance tests to incorporate
recent POSIX rulings and to match our strerror guarantees.
* doc/posix-functions/strerror_r.texi (strerror_r): Document this.

Signed-off-by: Eric Blake 
---

I've updated this patch, as well as tested this series on:

glibc 2.13, Solaris 10, FreeBSD 8.2, AIX 5.1, Irix 6.5, HP-UX 10.20
(not using the native strerror_r, but I'm still working that),
cygwin (still failing, but that's coming).  mingw testing still
underway.  perror needs to be replaced in more situations (I'm
still hoping to get to that soon), but this is now far enough and
passing strerror/_r on enough platforms that I feel good pushing it,
and it also matches glibc.git behavior as of Uli's patch last week
to guarantee that strerror_r populates buf even on error.

 ChangeLog   |9 +++
 doc/posix-functions/strerror_r.texi |   18 --
 lib/strerror_r.c|  114 +-
 tests/test-strerror_r.c |   88 ---
 4 files changed, 118 insertions(+), 111 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 4ec6463..bc838bd 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2011-05-24  Eric Blake  
+
+   strerror_r: enforce POSIX recommendations
+   * lib/strerror_r.c (safe_copy): New helper method.
+   (strerror_r): Guarantee a non-empty string.
+   * tests/test-strerror_r.c (main): Enhance tests to incorporate
+   recent POSIX rulings and to match our strerror guarantees.
+   * doc/posix-functions/strerror_r.texi (strerror_r): Document this.
+
 2011-05-24  Jim Meyering  

test-perror2.c: avoid warning about unused variable
diff --git a/doc/posix-functions/strerror_r.texi 
b/doc/posix-functions/strerror_r.texi
index 21e4181..91026ef 100644
--- a/doc/posix-functions/strerror_r.texi
+++ b/doc/posix-functions/strerror_r.texi
@@ -51,15 +51,21 @@ strerror_r
 platforms:
 HP-UX 11.31.
 @item
-When the buffer is too small, this function does not fail, but instead
-truncates the result and returns 0 on some platforms:
-OSF/1 5.1.
+When the buffer is too small and the value is in range, this function
+does not fail, but instead truncates the result and returns 0 on some
+platforms:
+AIX 6.1, OSF/1 5.1.
+@item
+When the value is not in range or the buffer is too small, this
+function fails to leave a NUL-terminated string in the buffer on some
+platforms:
+glibc 2.13, FreeBSD 8.2.
 @end itemize

 Portability problems not fixed by Gnulib:
 @itemize
 @item
-When the buffer is too small, this function does not fail, but instead
-truncates the result and returns 0 on some platforms:
-AIX 6.1.
+Calling this function can clobber the buffer used by @code{strerror}
+on some platforms:
+Cygwin 1.7.9.
 @end itemize
diff --git a/lib/strerror_r.c b/lib/strerror_r.c
index f0b59c7..40ebc59 100644
--- a/lib/strerror_r.c
+++ b/lib/strerror_r.c
@@ -92,6 +92,30 @@ gl_lock_define_initialized(static, strerror_lock)

 #endif

+/* Copy as much of MSG into BUF as possible, without corrupting errno.
+   Return 0 if MSG fit in BUFLEN, otherwise return ERANGE.  */
+static int
+safe_copy (char *buf, size_t buflen, const char *msg)
+{
+  size_t len = strlen (msg);
+  int ret;
+
+  if (len < buflen)
+{
+  /* Although POSIX allows memcpy() to corrupt errno, we don't
+ know of any implementation where this is a real problem.  */
+  memcpy (buf, msg, len + 1);
+  ret = 0;
+}
+  else
+{
+  memcpy (buf, msg, buflen - 1);
+  buf[buflen - 1] = '\0';
+  ret = ERANGE;
+}
+  return ret;
+}
+

 int
 strerror_r (int errnum, char *buf, size_t buflen)
@@ -102,9 +126,10 @@ strerror_r (int errnum, char *buf, size_t buflen)
   if (buflen <= 1)
 {
   if (buflen)
-*buf = 0;
+*buf = '\0';
   return ERANGE;
 }
+  *buf = '\0';

 #if GNULIB_defined_ETXTBSY \
 || GNULIB_defined_ESOCK \
@@ -413,19 +438,7 @@ strerror_r (int errnum, char *buf, size_t buflen)
   }

 if (msg)
-  {
-int saved_errno = errno;
-size_t len = strlen (msg);
-int ret = ERANGE;
-
-if (len < buflen)
-  {
-memcpy (buf, msg, len + 1);
-ret = 0;
-  }
-errno = saved_errno;
-return ret;
-  }
+  return safe_copy (buf, buflen, msg);
   }
 #endif

@@ -441,6 +454,13 @@ strerror_r (int errnum, char *buf, size_t buflen)
   ret = __xpg_strerror_r (errnum, buf, buflen);
   if (ret < 0)
 ret = errno;
+  if (!*buf)
+{
+  /* glibc 2.13 would not touch buf on err, so we have to fall
+ back to GNU strerror_r which always returns a thread-safe
+ 

[PATCH 2/3] strerror_r: fix Solaris test failures

[Eric Blake]

Solaris 10 populates buf on EINVAL, but not on ERANGE.

* lib/strerror_r.c (strerror_r): Partially populate buf on ERANGE
failures.
* doc/posix-functions/strerror_r.texi (strerror_r): Document this.

Signed-off-by: Eric Blake 
---
 ChangeLog   |5 +
 doc/posix-functions/strerror_r.texi |2 +-
 lib/strerror_r.c|   23 +++
 3 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index bc838bd..80e9369 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
 2011-05-24  Eric Blake  

+   strerror_r: fix Solaris test failures
+   * lib/strerror_r.c (strerror_r): Partially populate buf on ERANGE
+   failures.
+   * doc/posix-functions/strerror_r.texi (strerror_r): Document this.
+
strerror_r: enforce POSIX recommendations
* lib/strerror_r.c (safe_copy): New helper method.
(strerror_r): Guarantee a non-empty string.
diff --git a/doc/posix-functions/strerror_r.texi 
b/doc/posix-functions/strerror_r.texi
index 91026ef..e0f19c0 100644
--- a/doc/posix-functions/strerror_r.texi
+++ b/doc/posix-functions/strerror_r.texi
@@ -59,7 +59,7 @@ strerror_r
 When the value is not in range or the buffer is too small, this
 function fails to leave a NUL-terminated string in the buffer on some
 platforms:
-glibc 2.13, FreeBSD 8.2.
+glibc 2.13, FreeBSD 8.2, Solaris 10.
 @end itemize

 Portability problems not fixed by Gnulib:
diff --git a/lib/strerror_r.c b/lib/strerror_r.c
index 40ebc59..f6ce8a3 100644
--- a/lib/strerror_r.c
+++ b/lib/strerror_r.c
@@ -27,12 +27,16 @@

 #include 

-# if GNULIB_defined_ESOCK /* native Windows platforms */
-#  if HAVE_WINSOCK2_H
-#   include 
-#  endif
+#if GNULIB_defined_ESOCK /* native Windows platforms */
+# if HAVE_WINSOCK2_H
+#  include 
 # endif
+#endif

+/* Reasonable buffer size that should never trigger ERANGE; if this
+   proves too small, we intentionally abort(), to remind us to fix
+   this value as well as strerror-impl.h.  */
+#define STACKBUF_LEN 256

 #if (__GLIBC__ >= 2 || defined __UCLIBC__ || defined __CYGWIN__) && 
HAVE___XPG_STRERROR_R /* glibc >= 2.3.4, cygwin >= 1.7.9 */

@@ -483,7 +487,18 @@ strerror_r (int errnum, char *buf, size_t buflen)
 ret = strerror_r (errnum, buf, buflen);
 }
 # else
+/* Solaris 10 does not populate buf on ERANGE.  */
 ret = strerror_r (errnum, buf, buflen);
+if (ret == ERANGE && !*buf)
+  {
+char stackbuf[STACKBUF_LEN];
+
+/* strerror-impl.h is also affected if our choice of stackbuf
+   size is not large enough.  */
+if (strerror_r (errnum, stackbuf, sizeof stackbuf) == ERANGE)
+  abort ();
+safe_copy (buf, buflen, stackbuf);
+  }
 # endif

 /* Some old implementations may return (-1, EINVAL) instead of EINVAL.  */
-- 
1.7.4.4

Re: strerror vs. threads

[Paul Eggert]

Having had to deal with too many dependencies when using gnulib
in GNU Emacs, I can sympathize with Sam Steingold: it'd be nicer
to not pull in lock.c, lock.h, threadlib.c merely because an
application wants to use strerror.  For example, I can't see Emacs using
the strerror module if this problem isn't addressed.

Eric already pointed out that this locking is only needed for
multithreaded apps.  Also, it strikes me that most applications don't
care whether perror modifies the strerror static buffer, and
if that's the reason behind this change, then
applications that don't care about this bug shouldn't need
to pull in strerror_r, much less the locking code.

Fixing this dependency creep will require some work and testing,
and that's not always trivial to do.  I hope someone (Sam, maybe?)
can help out with that.

test-intprops.c: many new warnings

[Jim Meyering]

Still trying the latest from gnulib via coreutils,
"make check" was inundated with new warnings.
Nearly 500 lines worth.

Of course, I could simply turn off the warnings and/or -Werror
when building in gnulib-tests/, but I'd rather not.
I'll defer "upgrading to the latest" for now ;-)

...
Making check in gnulib-tests
...
test-intprops.c: In function 'main':
test-intprops.c:142:3: error: comparison of unsigned expression < 0 is always 
false [-Werror=type-limits]
test-intprops.c:142:3: error: comparison of unsigned expression < 0 is always 
false [-Werror=type-limits]
test-intprops.c:142:3: error: comparison of unsigned expression < 0 is always 
false [-Werror=type-limits]
test-intprops.c:142:3: error: comparison of unsigned expression < 0 is always 
false [-Werror=type-limits]
test-intprops.c:143:3: error: comparison of unsigned expression < 0 is always 
false [-Werror=type-limits]
...
test-intprops.c:164:3: error: string length '4422' is greater than the length 
'4095' ISO C99 compilers are required to support [-Werror=overlength-strings]
test-intprops.c:166:3: error: string length '5865' is greater than the length 
'4095' ISO C99 compilers are required to support [-Werror=overlength-strings]
test-intprops.c:172:3: error: comparison of unsigned expression < 0 is always 
false [-Werror=type-limits]
test-intprops.c:172:3: error: comparison of unsigned expression < 0 is always 
false [-Werror=type-limits]
test-intprops.c:172:3: error: comparison of unsigned expression < 0 is always 
false [-Werror=type-limits]
test-intprops.c:172:3: error: comparison of unsigned expression < 0 is always 
false [-Werror=type-limits]
test-intprops.c:172:3: error: comparison of unsigned expression < 0 is always 
false [-Werror=type-limits]
test-intprops.c:172:3: error: comparison of unsigned expression < 0 is always 
false [-Werror=type-limits]
test-intprops.c:175:3: error: string length '4096' is greater than the length 
'4095' ISO C99 compilers are required to support [-Werror=overlength-strings]
test-intprops.c:178:3: error: comparison of unsigned expression < 0 is always 
false [-Werror=type-limits]
test-intprops.c:178:3: error: comparison of unsigned expression < 0 is always 
false [-Werror=type-limits]
...
test-intprops.c:192:3: error: comparison of unsigned expression < 0 is always 
false [-Werror=type-limits]
test-intprops.c:192:3: error: comparison of unsigned expression >= 0 is always 
true [-Werror=type-limits]
test-intprops.c:192:3: error: comparison of unsigned expression < 0 is always 
false [-Werror=type-limits]
...
test-intprops.c:215:3: error: string length '4605' is greater than the length 
'4095' ISO C99 compilers are required to support [-Werror=overlength-strings]
test-intprops.c:216:3: error: string length '4241' is greater than the length 
'4095' ISO C99 compilers are required to support [-Werror=overlength-strings]
test-intprops.c:219:3: error: comparison of unsigned expression < 0 is always 
false [-Werror=type-limits]
...
test-intprops.c:240:3: error: comparison of unsigned expression < 0 is always 
false [-Werror=type-limits]
test-intprops.c:240:3: error: string length '4143' is greater than the length 
'4095' ISO C99 compilers are required to support [-Werror=overlength-strings]
test-intprops.c:240:3: error: comparison of unsigned expression < 0 is always 
false [-Werror=type-limits]
...
test-intprops.c:241:3: error: string length '5235' is greater than the length 
'4095' ISO C99 compilers are required to support [-Werror=overlength-strings]
test-intprops.c:241:3: error: comparison of unsigned expression < 0 is always 
false [-Werror=type-limits]
...
test-intprops.c:241:3: error: comparison of unsigned expression < 0 is always 
false [-Werror=type-limits]
test-intprops.c:241:3: error: string length '4521' is greater than the length 
'4095' ISO C99 compilers are required to support [-Werror=overlength-strings]
test-intprops.c:245:3: error: comparison of unsigned expression < 0 is always 
false [-Werror=type-limits]
test-intprops.c:245:3: error: comparison of unsigned expression < 0 is always 
false [-Werror=type-limits]
...
cc1: all warnings being treated as errors

make[5]: *** [test-intprops.o] Error 1
make[5]: Leaving directory `/h/j/w/co/cu/gnulib-tests'
make[4]: *** [check-am] Error 2
make[4]: Leaving directory `/h/j/w/co/cu/gnulib-tests'
make[3]: *** [check-recursive] Error 1
make[3]: Leaving directory `/h/j/w/co/cu/gnulib-tests'
make[2]: *** [check] Error 2
make[2]: Leaving directory `/h/j/w/co/cu/gnulib-tests'
make[1]: *** [check-recursive] Error 1
make[1]: Leaving directory `/h/j/w/co/cu'
make: *** [check] Error 2

Re: strerror vs. threads [was: new files imported without new modules added]

[Eric Blake]

On 05/24/2011 01:27 PM, Sam Steingold wrote:
>> * Eric Blake  [2011-05-24 12:24:34 -0600]:
>>
>> strerror(-1) in thread 1
>> strerror(-2) in thread 2
> 
> thanks for the explanation.
> 
> My further questions are:
> you are not using the standard win32 FormatMessage() function.
> how do you hangle the gazillion windows error messages?

strerror[_r]() only handles the messages that map to errno values.  And
if you use sockets, gnulib already maps quite a few windows errors to
errno values.

If you care about windows errors that do not map to errno values, you
are probably writing a windows-specific program, at which point gnulib
isn't going to help you.

> (same goes for system-supplied strerror - is it ever used?)

If the underlying strerror_r translates, then yes, we have a mismatch
where some, but not all, errno values are translated to the user's
locale.  But this question was already asked last week:
http://lists.gnu.org/archive/html/bug-gnulib/2011-05/msg00421.html

and if someone does the work, we could certainly support translating the
gnulib-added errno values as well.

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



signature.asc
Description: OpenPGP digital signature

[PATCH] test-perror2.c: avoid warning about unused variable

[Jim Meyering]

Trying out the latest from gnulib via coreutils,
I saw this new warning/failure:

  ...
  Making check in gnulib-tests
  ...
  test-perror2.c: In function 'main':
  test-perror2.c:42:9: error: unused variable 'fp' [-Werror=unused-variable]
  cc1: all warnings being treated as errors

  make[5]: *** [test-perror2.o] Error 1
  make[5]: *** Waiting for unfinished jobs

I've just pushed this as obvious:

>From 734bedd126553e3619936f4fa84bf4854b7c40d6 Mon Sep 17 00:00:00 2001
From: Jim Meyering 
Date: Tue, 24 May 2011 21:28:46 +0200
Subject: [PATCH] test-perror2.c: avoid warning about unused variable

* tests/test-perror2.c (main): Remove declaration of unused "fp".
---
 ChangeLog|5 +
 tests/test-perror2.c |2 --
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index f606d69..4ec6463 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2011-05-24  Jim Meyering  
+
+   test-perror2.c: avoid warning about unused variable
+   * tests/test-perror2.c (main): Remove declaration of unused "fp".
+
 2011-05-24  Eric Blake  

perror: avoid spurious test failure on HP-UX
diff --git a/tests/test-perror2.c b/tests/test-perror2.c
index d8e0ec5..3aab640 100644
--- a/tests/test-perror2.c
+++ b/tests/test-perror2.c
@@ -37,8 +37,6 @@ static FILE *myerr;
 int
 main (void)
 {
-  FILE *fp;
-
   /* We change fd 2 later, so save it in fd 10.  */
   if (dup2 (STDERR_FILENO, BACKUP_STDERR_FILENO) != BACKUP_STDERR_FILENO
   || (myerr = fdopen (BACKUP_STDERR_FILENO, "w")) == NULL)
--
1.7.5.2.585.gfbd48

Re: strerror vs. threads [was: new files imported without new modules added]

[Sam Steingold]

> * Eric Blake  [2011-05-24 12:24:34 -0600]:
>
> strerror(-1) in thread 1
> strerror(-2) in thread 2

thanks for the explanation.

My further questions are:
you are not using the standard win32 FormatMessage() function.
how do you hangle the gazillion windows error messages?
(same goes for system-supplied strerror - is it ever used?)

thanks!

-- 
Sam Steingold (http://sds.podval.org/) on CentOS release 5.6 (Final) X 
11.0.60900031
http://www.memritv.org http://mideasttruth.com http://palestinefacts.org
http://iris.org.il http://openvotingconsortium.org http://honestreporting.com
Murphy's Law was probably named after the wrong guy.

Re: [PATCH 3/3] perror: work around FreeBSD bug

[Eric Blake]

On 05/20/2011 11:59 AM, Eric Blake wrote:
> POSIX requires that 'errno = 0; perror ("")' print the same message
> as strerror(0), but this failed if we were replacing strerror to work
> around the FreeBSD bug of treating 0 as a failure.
> 
> * m4/perror.m4 (gl_FUNC_PERROR): Also replace perror if strerror_r
> is broken.  Move AC_LIBOBJ...
> * modules/perror (configure.ac): Here.
> * doc/posix-functions/perror.texi (perror): Document this.
> * tests/test-perror2.c (main): Enhance test.

This patch is incomplete - I'm still seeing perror failures on at least
AIX and Irix, due to strerror_r but not perror being replaced.  I'm not
quite sure of the right m4 fix to make, but hopefully I can fix this
soon (first, though, I plan on completing my strerror_r work).

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



signature.asc
Description: OpenPGP digital signature

fprintf failures on read-only streams

[Eric Blake]

Just last week, glibc fixed a bug where vfprintf failed to set ferror()
when returning EOF due to an attempt to output to a read-only stream:

http://sourceware.org/git/?p=glibc.git;a=commitdiff;h=aec84f5395

Should we update fprintf-posix and friends to detect this bug and work
around it on older glibc?  And at any rate, we probably need to update
docs/posix-functions/[v][f]printf to document it.

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



signature.asc
Description: OpenPGP digital signature

Re: new files imported without new modules added

[Eric Blake]

On 05/24/2011 10:32 AM, Sam Steingold wrote:
> I think this is wrong. I do not want strerror_r. I did not ask for it
> and there is no need for it in any module I asked for.

Umm, but there is - the strerror module needs it, to guarantee that it
outputs the same replacement errno strings, without code duplication
(although as I've already pointed out, we could probably refactor
strerror.c and strerror_r.c to use a common third file for those
strings, rather than making strerror use strerror_r).

> You are making the use of gnulib a very risky proposition.
> I do _NOT_ want to bundle the whole of gnu libc with clisp.

Sounds like you would benefit from the proposed libposix project, which
would alleviate the need for any POSIX-replaced functions from gnulib to
appear in your source repository.  Personally, I haven't been following
the progress of that much, but I know there are still some hurdles to
overcome before it is polished enough to release, so patches welcome.

> You are making sure that I have to pull in more and more files every
> time I update the gnulib files.

That's somewhat to be expected - gnulib is the collection point for all
known portability workarounds, and we seem to be learning about more and
more needed workarounds as our portability increases.  If you are
worried about the size of your source repository, you could omit gnulib
files from it (the git submodule approach used by coreutils and others
has been working quite well); although that doesn't really decrease the
size of your source tarballs any.

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



signature.asc
Description: OpenPGP digital signature

Re: strerror vs. threads

[Eric Blake]

On 05/24/2011 12:52 PM, Simon Josefsson wrote:
>> POSIX explicitly allows strerror to use a static buffer, and that's
>> _exactly_ what gnulib's implementation does on out-of-range input.
>> Which means that "Unknown error (-1)" of thread 1 and "Unknown error
>> (-2)" of thread 2 are calling sprintf on the same memory at the same
>> time, and you will get indeterminate results.
> 
> Which begs the question why the error messages needs to be different for
> different unknown errors?  Is that required by POSIX?

Not required, but heavily recommended, and guaranteed by several common
platforms.  And since "Error: " is much more confusing than "Error:
Unknown error -1", especially at tracking down a bug at why errno is set
to some weird value, it was worth documenting that we guarantee that
behavior as one of the points of the gnulib strerror replacement.

If you don't ever pass out-of-range errno values to strerror, then you
probably don't care about thread-safety of the out-of-range buffer, but
there is still the question of whether all existing implementations are
thread-safe even on in-range errno values.

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



signature.asc
Description: OpenPGP digital signature

Re: strerror vs. threads [was: new files imported without new modules added]

[Simon Josefsson]

Eric Blake  writes:

> On 05/24/2011 12:06 PM, Sam Steingold wrote:
>>> * Eric Blake  [2011-05-24 10:54:20 -0600]:
>>>
>>> Are you multi-threaded?  Then you are suffering from a data race.
>> 
>> I am sorry, I am afraid I am out of my depth.
>> Why is this function "suffering from a data race"?
>> 
>> const char *strerror (int e) {
>>   switch (e) {
>> case EINPROGRESS: return "Operation now in progress";
>> case EALREADY: return "Operation already in progress";
>> ...
>>   }
> ...
>   {
> static char const fmt[] = "Unknown error (%d)";
> verify (sizeof (buf) >= sizeof (fmt) + INT_STRLEN_BOUND (n));
> sprintf (buf, fmt, n);
>
>> }
>
> Try:
>
> strerror(-1) in thread 1
> strerror(-2) in thread 2
>
> POSIX explicitly allows strerror to use a static buffer, and that's
> _exactly_ what gnulib's implementation does on out-of-range input.
> Which means that "Unknown error (-1)" of thread 1 and "Unknown error
> (-2)" of thread 2 are calling sprintf on the same memory at the same
> time, and you will get indeterminate results.

Which begs the question why the error messages needs to be different for
different unknown errors?  Is that required by POSIX?

/Simon

strerror vs. threads [was: new files imported without new modules added]

[Eric Blake]

On 05/24/2011 12:06 PM, Sam Steingold wrote:
>> * Eric Blake  [2011-05-24 10:54:20 -0600]:
>>
>> Are you multi-threaded?  Then you are suffering from a data race.
> 
> I am sorry, I am afraid I am out of my depth.
> Why is this function "suffering from a data race"?
> 
> const char *strerror (int e) {
>   switch (e) {
> case EINPROGRESS: return "Operation now in progress";
> case EALREADY: return "Operation already in progress";
> ...
>   }
...
  {
static char const fmt[] = "Unknown error (%d)";
verify (sizeof (buf) >= sizeof (fmt) + INT_STRLEN_BOUND (n));
sprintf (buf, fmt, n);

> }

Try:

strerror(-1) in thread 1
strerror(-2) in thread 2

POSIX explicitly allows strerror to use a static buffer, and that's
_exactly_ what gnulib's implementation does on out-of-range input.
Which means that "Unknown error (-1)" of thread 1 and "Unknown error
(-2)" of thread 2 are calling sprintf on the same memory at the same
time, and you will get indeterminate results.  And other implementations
might share a single buffer across threads even for in-range messages
(at least FreeBSD strerror() overwrites the contents at the pointer from
prior calls, rather than returning distinct pointers, when calling
strerror(EACCES); strerror(ERANGE) in the same thread, although I
haven't yet looked at BSD sources to see whether that is thread-local or
global storage).

Since POSIX explicitly documents that strerror() need not be threadsafe,
you should not use it in multi-threaded programs.  Just because glibc's
version happens to be threadsafe does not mean that all other
implementations are threadsafe.

strerror_l() is required to be thread-safe (that is, POSIX requires that
it use thread-local buffers for any computed values), but it is not as
widely implemented and not yet available in gnulib.

You only get true thread safety by passing in the storage, as is the
case of strerror_r.

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



signature.asc
Description: OpenPGP digital signature

Re: new files imported without new modules added

[Sam Steingold]

> * Eric Blake  [2011-05-24 10:54:20 -0600]:
>
> Are you multi-threaded?  Then you are suffering from a data race.

I am sorry, I am afraid I am out of my depth.
Why is this function "suffering from a data race"?

const char *strerror (int e) {
  switch (e) {
case EINPROGRESS: return "Operation now in progress";
case EALREADY: return "Operation already in progress";
...
  }
}

-- 
Sam Steingold (http://sds.podval.org/) on CentOS release 5.6 (Final) X 
11.0.60900031
http://honestreporting.com http://iris.org.il http://jihadwatch.org
http://pmw.org.il http://camera.org http://thereligionofpeace.com
The only time you have too much fuel is when you're on fire.

Re: request: pointer to the docs in the module file

[Sam Steingold]

> * Eric Blake  [2011-05-24 11:37:40 -0600]:
> On 05/24/2011 11:26 AM, Sam Steingold wrote:
>> How do I find out where a module is documented?
>> E.g., when I discovered the getloadavg module, I had to do
>> find gnulib/ -name getload\*
>> to find gnulib/doc/glibc-functions/getloadavg.texi.
>
> If it is a POSIX function, it is documented in
> gnulib/doc/posix-functions.  If it is a glibc extension, it is
> documented in gnulib/doc/glibc-functions.  Otherwise, it is a gnulib
> invention, and we haven't been very consistent at where that
> documentation lives.

Yes, thanks, you (or someone else) already told me that at least once,
and I am using the advice, even though I usually cannot tell right away
whether a specific function is posix or glibc (e.g., I thought that
mkdtemp was not in posix) - which is why I am asking for the link.

>> It would be nice if there were a pointer in gnulib/modules/getloadavg,
>> preferably a URL with the up-to-date (updated nightly) doc generated from
>> the texi file.
>
> I would prefer not in the modules file itself (that seems like a
> maintenance burden to have to manually maintain an extra link), but
> perhaps in the generated web page that describes each module.

I guess I am using gnulib wrong.
Suppose I want to replace my very own maze of #ifdefs surrounding a call
to a posix function foo() with a nice gnulib module foo.
Right now I look for modules/foo and, if I see that it does not pull in
too many dependencies, I try to find the texi file to see if it solves
my portability problems. This is why I want a link in the modules/foo
file to foo.texi (or foo.html) so that I do not have to run a find.
What is the right way?

>> Also, mkdir.texi says:
>> -
>> On Windows platforms (excluding Cygwin), this function is called 
>> @code{_mkdir}
>> and takes only one argument.  The fix (without Gnulib) is to define a macro
>> like this:
>> -
>> what does this mean?
>> I need to use this macro if I am not using Gnulib?
>> I need to define the macro even if I am using Gnulib?
>
> It means that you either use the gnulib module (and don't worry about
> anything extra), or you can be lighter-weight and use that listed
> workaround instead of the gnulib module.  So only define that macro only
> if you are not using gnulib.

So, if I were using the mkdir module (which I won't because it pulls
dirname-lgpl which pulls a bunch of other modules), I can use mkdir with
2 arguments on mingw too?
But which header file will defined mkdir with 2 arguments?
The mkdir module does not seem to export any header files.
Oh wait - it will replace mkdir with a _function_ rpl_mkdir, right?

> That is, docs/posix-functions is intended to be a catch-all for _all_
> portability problems, not just those fixed by gnulib, but tends to be
> biased towards gnulib solutions.

I am sorry, I can parse this sentence just fine, but I cannot extract
the meaning.  Remember, English is not my native language.

--
Sam Steingold (http://sds.podval.org/) on CentOS release 5.6 (Final) X 
11.0.60900031
http://truepeace.org http://ffii.org http://openvotingconsortium.org
http://www.memritv.org http://honestreporting.com http://jihadwatch.org
As a computer, I find your faith in technology amusing.

Re: sed hangs on solaris

[Sam Steingold]

> * Paul Eggert  [2011-05-03 13:52:21 -0700]:
>
> On 05/03/11 13:26, Sam Steingold wrote:
>> note that some pipelines appear to succeed before the hanging:
>
> Yes.  If it is a kernel bug, quite possibly it's timing-dependent,
> and I wouldn't be surprised if it's not that common.

isn't gnulib's raison d'etre to work around system bugs and deficiencies? :-)
how about an --avoid-pipes option to something (configure?) which would
let me avoid editing the generated Makefile by hand?
Right now I need to apply the appended patch after each "make gllib".

-- 
Sam Steingold (http://sds.podval.org/) on CentOS release 5.6 (Final) X 
11.0.60900031
http://thereligionofpeace.com http://ffii.org http://openvotingconsortium.org
http://camera.org http://jihadwatch.org http://www.memritv.org
There are no answers, only cross references.


--- gllib/Makefile.orig 2011-05-18 11:43:58.838261985 -0400
+++ gllib/Makefile  2011-05-18 11:46:31.150273797 -0400
@@ -2074,7 +2074,6 @@
 #  when the system doesn't have one.
 unistd.h: unistd.in.h $(top_builddir)/config.status $(CXXDEFS_H) 
$(ARG_NONNULL_H) $(WARN_ON_USE_H)
$(AM_V_GEN)rm -f $@-t $@ && \
-   { echo '/* DO NOT EDIT! GENERATED AUTOMATICALLY! */'; \
  sed -e 's|@''HAVE_UNISTD_H''@|$(HAVE_UNISTD_H)|g' \
  -e 's|@''INCLUDE_NEXT''@|$(INCLUDE_NEXT)|g' \
  -e 's|@''PRAGMA_SYSTEM_HEADER''@|#pragma GCC system_header|g' \
@@ -2124,7 +2123,7 @@
  -e 's|@''GNULIB_UNLINKAT''@|$(GNULIB_UNLINKAT)|g' \
  -e 's|@''GNULIB_USLEEP''@|$(GNULIB_USLEEP)|g' \
  -e 's|@''GNULIB_WRITE''@|$(GNULIB_WRITE)|g' \
- < $(srcdir)/unistd.in.h | \
+ < $(srcdir)/unistd.in.h > z
  sed -e 's|@''HAVE_CHOWN''@|$(HAVE_CHOWN)|g' \
  -e 's|@''HAVE_DUP2''@|$(HAVE_DUP2)|g' \
  -e 's|@''HAVE_DUP3''@|$(HAVE_DUP3)|g' \
@@ -2163,7 +2162,7 @@
  -e 's|@''HAVE_DECL_TTYNAME_R''@|$(HAVE_DECL_TTYNAME_R)|g' \
  -e 's|@''HAVE_OS_H''@|$(HAVE_OS_H)|g' \
  -e 's|@''HAVE_SYS_PARAM_H''@|$(HAVE_SYS_PARAM_H)|g' \
- | \
+ < z > y
  sed -e 's|@''REPLACE_CHOWN''@|$(REPLACE_CHOWN)|g' \
  -e 's|@''REPLACE_CLOSE''@|$(REPLACE_CLOSE)|g' \
  -e 's|@''REPLACE_DUP''@|$(REPLACE_DUP)|g' \
@@ -2194,8 +2193,8 @@
  -e 
's|@''UNISTD_H_HAVE_WINSOCK2_H_AND_USE_SOCKETS''@|$(UNISTD_H_HAVE_WINSOCK2_H_AND_USE_SOCKETS)|g'
 \
  -e '/definitions of _GL_FUNCDECL_RPL/r $(CXXDEFS_H)' \
  -e '/definition of _GL_ARG_NONNULL/r $(ARG_NONNULL_H)' \
- -e '/definition of _GL_WARN_ON_USE/r $(WARN_ON_USE_H)'; \
-   } > $@-t && \
+ -e '/definition of _GL_WARN_ON_USE/r $(WARN_ON_USE_H)' \
+   < y > $@-t && \
mv $@-t $@
 
 unitypes.h: unitypes.in.h
@@ -2224,7 +2223,6 @@
 # version does not work standalone.
 wchar.h: wchar.in.h $(top_builddir)/config.status $(CXXDEFS_H) 
$(ARG_NONNULL_H) $(WARN_ON_USE_H)
$(AM_V_GEN)rm -f $@-t $@ && \
-   { echo '/* DO NOT EDIT! GENERATED AUTOMATICALLY! */'; \
  sed -e 's|@''INCLUDE_NEXT''@|$(INCLUDE_NEXT)|g' \
  -e 's|@''PRAGMA_SYSTEM_HEADER''@|#pragma GCC system_header|g' \
  -e 's|@''PRAGMA_COLUMNS''@||g' \
@@ -2270,7 +2268,7 @@
  -e 's|@''GNULIB_WCSSTR''@|$(GNULIB_WCSSTR)|g' \
  -e 's|@''GNULIB_WCSTOK''@|$(GNULIB_WCSTOK)|g' \
  -e 's|@''GNULIB_WCSWIDTH''@|$(GNULIB_WCSWIDTH)|g' \
- < $(srcdir)/wchar.in.h | \
+ < $(srcdir)/wchar.in.h > z
  sed -e 's|@''HAVE_WINT_T''@|$(HAVE_WINT_T)|g' \
  -e 's|@''HAVE_BTOWC''@|$(HAVE_BTOWC)|g' \
  -e 's|@''HAVE_MBSINIT''@|$(HAVE_MBSINIT)|g' \
@@ -2311,7 +2309,7 @@
  -e 's|@''HAVE_WCSWIDTH''@|$(HAVE_WCSWIDTH)|g' \
  -e 's|@''HAVE_DECL_WCTOB''@|$(HAVE_DECL_WCTOB)|g' \
  -e 's|@''HAVE_DECL_WCWIDTH''@|$(HAVE_DECL_WCWIDTH)|g' \
- | \
+ < z > y
  sed -e 's|@''REPLACE_MBSTATE_T''@|$(REPLACE_MBSTATE_T)|g' \
  -e 's|@''REPLACE_BTOWC''@|$(REPLACE_BTOWC)|g' \
  -e 's|@''REPLACE_WCTOB''@|$(REPLACE_WCTOB)|g' \
@@ -2327,8 +2325,8 @@
  -e 's|@''REPLACE_WCSWIDTH''@|$(REPLACE_WCSWIDTH)|g' \
  -e '/definitions of _GL_FUNCDECL_RPL/r $(CXXDEFS_H)' \
  -e '/definition of _GL_ARG_NONNULL/r $(ARG_NONNULL_H)' \
- -e '/definition of _GL_WARN_ON_USE/r $(WARN_ON_USE_H)'; \
-   } > $@-t && \
+ -e '/definition of _GL_WARN_ON_USE/r $(WARN_ON_USE_H)' \
+   < y > $@-t && \
mv $@-t $@
 
 # We need the following in order to create  when the system

Re: request: pointer to the docs in the module file

[Eric Blake]

On 05/24/2011 11:26 AM, Sam Steingold wrote:
> Hi,
> 
> How do I find out where a module is documented?
> E.g., when I discovered the getloadavg module, I had to do
> find gnulib/ -name getload\*
> to find gnulib/doc/glibc-functions/getloadavg.texi.

If it is a POSIX function, it is documented in
gnulib/doc/posix-functions.  If it is a glibc extension, it is
documented in gnulib/doc/glibc-functions.  Otherwise, it is a gnulib
invention, and we haven't been very consistent at where that
documentation lives.

> It would be nice if there were a pointer in gnulib/modules/getloadavg,
> preferably a URL with the up-to-date (updated nightly) doc generated from
> the texi file.

I would prefer not in the modules file itself (that seems like a
maintenance burden to have to manually maintain an extra link), but
perhaps in the generated web page that describes each module.

> 
> Also, in the MODULES.html file:
> 1. the dependencies should be links

Patches to the generating script are welcome.

> 2. the links to files (e.g., argz.c et al) should specify the file type
> text/plain so that firefox does not ask me how to open it.

Don't know whether that is possible - that's more of a web server issue.

> say something like this:
> -
> Portability problems fixed by Gnulib:
> @itemize
> @end itemize
> 
> Portability problems not fixed by Gnulib:
> @itemize
> @item
> This function is missing on some platforms: ...
> @end itemize
> -
> if no "portability problems" are fixed, what does this module do?

That means we don't yet have a module that fixes the known portability
problems in that function.  Patches welcome.

> 
> Also, mkdir.texi says:
> -
> On Windows platforms (excluding Cygwin), this function is called @code{_mkdir}
> and takes only one argument.  The fix (without Gnulib) is to define a macro
> like this:
> -
> what does this mean?
> I need to use this macro if I am not using Gnulib?
> I need to define the macro even if I am using Gnulib?

It means that you either use the gnulib module (and don't worry about
anything extra), or you can be lighter-weight and use that listed
workaround instead of the gnulib module.  So only define that macro only
if you are not using gnulib.

That is, docs/posix-functions is intended to be a catch-all for _all_
portability problems, not just those fixed by gnulib, but tends to be
biased towards gnulib solutions.

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



signature.asc
Description: OpenPGP digital signature

request: pointer to the docs in the module file

[Sam Steingold]

Hi,

How do I find out where a module is documented?
E.g., when I discovered the getloadavg module, I had to do
find gnulib/ -name getload\*
to find gnulib/doc/glibc-functions/getloadavg.texi.
It would be nice if there were a pointer in gnulib/modules/getloadavg,
preferably a URL with the up-to-date (updated nightly) doc generated from
the texi file.

Also, in the MODULES.html file:
1. the dependencies should be links
2. the links to files (e.g., argz.c et al) should specify the file type
text/plain so that firefox does not ask me how to open it.
3. should contain the aforementioned link to the doc generated from
*.texi

Also, some texi docs, e.g.,
   getutxid.texi
   getuid.texi
   getpwnam.texi
   getrusage.texi
say something like this:
-
Portability problems fixed by Gnulib:
@itemize
@end itemize

Portability problems not fixed by Gnulib:
@itemize
@item
This function is missing on some platforms: ...
@end itemize
-
if no "portability problems" are fixed, what does this module do?

Also, mkdir.texi says:
-
On Windows platforms (excluding Cygwin), this function is called @code{_mkdir}
and takes only one argument.  The fix (without Gnulib) is to define a macro
like this:
-
what does this mean?
I need to use this macro if I am not using Gnulib?
I need to define the macro even if I am using Gnulib?

Thanks.

-- 
Sam Steingold (http://sds.podval.org/) on CentOS release 5.6 (Final) X 
11.0.60900031
http://iris.org.il http://mideasttruth.com http://honestreporting.com
http://www.memritv.org http://ffii.org http://dhimmi.com http://pmw.org.il
non-smoking section in a restaurant == non-peeing section in a swimming pool

Re: new files imported without new modules added

[Eric Blake]

On 05/24/2011 10:45 AM, Sam Steingold wrote:
>> * Eric Blake  [2011-05-24 10:32:16 -0600]:
>>
>>> I do _not_ want strerror_r.
>>
>> Why not?
> 
> because I copy away the strerror return value right away.

Are you multi-threaded?  Then you are suffering from a data race.  Are
you single threaded?  Then you shouldn't care if strerror had to use
strerror_r under the hood for its implementation (that's how glibc does
it), but you do have a (minor) point that configure is needlessly larger
in checking for strerror_r compliance.  Yes, strerror() is an
easier-to-use interface than sterror_r, but it is _only_ safe in
single-threaded programs.

Assuming you are single-threaded, your complaint is now: why does gnulib
take two files to implement sterror where it used to take one?  And the
answer is because when gnulib only used one file (that is, strerror.c
provided the gnulib replacement strings), then gnulib's perror and
strerror_r modules were not standards-compliant, whether or not you used
them.  Yes, we could make strerror.c and strerror_r.c both implement the
gnulib replacement strings, but that adds bulk.

So about the only other thing we can do is make both strerror.c and
strerror_r.c call into a third file that implements the gnulib
replacement strings, and thus make it so that strerror no longer
directly depends on strerror_r.c; patches welcome.

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



signature.asc
Description: OpenPGP digital signature

Re: new files imported without new modules added

[Sam Steingold]

> * Eric Blake  [2011-05-24 10:32:16 -0600]:
>
>> I do _not_ want strerror_r.
>
> Why not?

because I copy away the strerror return value right away.

-- 
Sam Steingold (http://sds.podval.org/) on CentOS release 5.6 (Final) X 
11.0.60900031
http://mideasttruth.com http://thereligionofpeace.com http://ffii.org
http://dhimmi.com http://openvotingconsortium.org http://palestinefacts.org
If you need a helping hand, just remember that you already have two.

Re: new files imported without new modules added

[Sam Steingold]

> * Eric Blake  [2011-05-24 10:21:16 -0600]:
> On 05/24/2011 10:18 AM, Eric Blake wrote:
>> On 05/24/2011 10:12 AM, Sam Steingold wrote:
>>> I updated gnulib and found that it now wants to add these files to
>>> CLISP:
>>>
>>> src/gllib/glthread/lock.c
>>> src/gllib/glthread/lock.h
>>> src/gllib/glthread/threadlib.c
>>> src/gllib/strerror-impl.h
>>> src/gllib/strerror_r.c
>>> src/glm4/strerror_r.m4
>>>
>>> I did not import any new modules.
>>> what has happened?
>
> That means that strerror-impl.h, strerror_r.c, and strerror_r.m4 will
> still be needed, but it would get rid of the three glthread files.

I think this is wrong. I do not want strerror_r. I did not ask for it
and there is no need for it in any module I asked for.

You are making the use of gnulib a very risky proposition.
I do _NOT_ want to bundle the whole of gnu libc with clisp.
You are making sure that I have to pull in more and more files every
time I update the gnulib files.

-- 
Sam Steingold (http://sds.podval.org/) on CentOS release 5.6 (Final) X 
11.0.60900031
http://ffii.org http://memri.org http://www.memritv.org http://pmw.org.il
http://iris.org.il http://dhimmi.com http://mideasttruth.com
Lisp is a way of life.  C is a way of death.

Re: new files imported without new modules added

[Eric Blake]

On 05/24/2011 10:27 AM, Sam Steingold wrote:
>> * Eric Blake  [2011-05-24 10:18:05 -0600]:
>> On 05/24/2011 10:12 AM, Sam Steingold wrote:
>>> I updated gnulib and found that it now wants to add these files to
>>> CLISP:
>>>
>>> src/gllib/glthread/lock.c
>>> src/gllib/glthread/lock.h
>>> src/gllib/glthread/threadlib.c
>>> src/gllib/strerror-impl.h
>>> src/gllib/strerror_r.c
>>> src/glm4/strerror_r.m4
>>>
>>> I did not import any new modules.
>>> what has happened?
>>
>> strerror and perror now depend on strerror_r, and strerror_r depends
>> on locking; this was done in order to fix perror bugs (perror is not
>> allowed to modify the strerror static buffer).
> 
> Is there a way to avoid this dependency creep?

Not without breaking strerror_r and perror for other users.

> I do _not_ want strerror_r.

Why not?  If you are multi-threaded, then you absolutely want strerror_r
(and _not_ strerror), if you care at all about thread-safety.  And if
you are single-threaded, you are still free to use the much nicer
strerror() interface without having to go through the strerror_r hoops;
but we would still prefer implementing strerror on top of strerror_r
under the hood, to avoid duplication of code (otherwise, both strerror
and sterror_r have to duplicate the gnulib replacement error messages).

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



signature.asc
Description: OpenPGP digital signature

Re: new files imported without new modules added

[Sam Steingold]

> * Eric Blake  [2011-05-24 10:18:05 -0600]:
> On 05/24/2011 10:12 AM, Sam Steingold wrote:
>> I updated gnulib and found that it now wants to add these files to
>> CLISP:
>> 
>> src/gllib/glthread/lock.c
>> src/gllib/glthread/lock.h
>> src/gllib/glthread/threadlib.c
>> src/gllib/strerror-impl.h
>> src/gllib/strerror_r.c
>> src/glm4/strerror_r.m4
>> 
>> I did not import any new modules.
>> what has happened?
>
> strerror and perror now depend on strerror_r, and strerror_r depends
> on locking; this was done in order to fix perror bugs (perror is not
> allowed to modify the strerror static buffer).

Is there a way to avoid this dependency creep?
I do _not_ want strerror_r.

-- 
Sam Steingold (http://sds.podval.org/) on CentOS release 5.6 (Final) X 
11.0.60900031
http://dhimmi.com http://palestinefacts.org http://truepeace.org
http://camera.org http://www.memritv.org http://thereligionofpeace.com
As a computer, I find your faith in technology amusing.

Re: [PATCH] utimensat: do not reference an out-of-scope buffer

[Eric Blake]

On 05/24/2011 10:25 AM, Jim Meyering wrote:
> Another coverity-spotted bug.
> Eric, ok to push?
> 
>>From 6dc42e2d25df9c84b335062bad9beb0a7319647b Mon Sep 17 00:00:00 2001
> From: Jim Meyering 
> Date: Tue, 24 May 2011 18:24:24 +0200
> Subject: [PATCH] utimensat: do not reference an out-of-scope buffer
> 
> Otherwise, with __linux__ defined, "times" would point to a buffer, "ts"
> declared in an inner scope, yet "times" would be dereferenced outside
> the scope in which "ts" was valid.
> * lib/utimensat.c (rpl_utimensat) [__linux__]: Move the declaration
> of ts[2] "out/up", so that the use of aliased "times" (via "times = ts;")
> does not end up referencing an out-of-scope "ts"

Gotta love coverity for catching this!  Please push.

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



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 1/2] tests: fix logic bug in init.sh

[Jim Meyering]

Eric Blake wrote:
> If the shell test loop first finds a marginal then a good shell, the
> variable $gl_set_x_corrupts_stderr is still set to true and needlessly
> drops $VERBOSE logging.
>
> * tests/init.sh: (gl_set_x_corrupts_stderr_): Clear for successful
> shell.

Perfect.  Thanks!

[PATCH] utimensat: do not reference an out-of-scope buffer

[Jim Meyering]

Another coverity-spotted bug.
Eric, ok to push?

>From 6dc42e2d25df9c84b335062bad9beb0a7319647b Mon Sep 17 00:00:00 2001
From: Jim Meyering 
Date: Tue, 24 May 2011 18:24:24 +0200
Subject: [PATCH] utimensat: do not reference an out-of-scope buffer

Otherwise, with __linux__ defined, "times" would point to a buffer, "ts"
declared in an inner scope, yet "times" would be dereferenced outside
the scope in which "ts" was valid.
* lib/utimensat.c (rpl_utimensat) [__linux__]: Move the declaration
of ts[2] "out/up", so that the use of aliased "times" (via "times = ts;")
does not end up referencing an out-of-scope "ts"
---
 ChangeLog   |8 
 lib/utimensat.c |5 -
 2 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index b7be3f3..4db03d6 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
 2011-05-24  Jim Meyering  

+   utimensat: do not reference an out-of-scope buffer
+   Otherwise, with __linux__ defined, "times" would point to a buffer, "ts"
+   declared in an inner scope, yet "times" would be dereferenced outside
+   the scope in which "ts" was valid.
+   * lib/utimensat.c (rpl_utimensat) [__linux__]: Move the declaration
+   of ts[2] "out/up", so that the use of aliased "times" (via "times = 
ts;")
+   does not end up referencing an out-of-scope "ts"
+
opendir-safer.c: don't clobber errno; don't close negative FD
* lib/opendir-safer.c (opendir_safer):
[HAVE_FDOPENDIR || GNULIB_FDOPENDIR]: Don't close a negative
diff --git a/lib/utimensat.c b/lib/utimensat.c
index e63692a..5a55e64 100644
--- a/lib/utimensat.c
+++ b/lib/utimensat.c
@@ -48,6 +48,10 @@ int
 rpl_utimensat (int fd, char const *file, struct timespec const times[2],
int flag)
 {
+# ifdef __linux__
+  struct timespec ts[2];
+# endif
+
   /* See comments in utimens.c for details.  */
   static int utimensat_works_really; /* 0 = unknown, 1 = yes, -1 = no.  */
   if (0 <= utimensat_works_really)
@@ -55,7 +59,6 @@ rpl_utimensat (int fd, char const *file, struct timespec 
const times[2],
   int result;
 # ifdef __linux__
   struct stat st;
-  struct timespec ts[2];
   /* As recently as Linux kernel 2.6.32 (Dec 2009), several file
  systems (xfs, ntfs-3g) have bugs with a single UTIME_OMIT,
  but work if both times are either explicitly specified or
--
1.7.5.2.585.gfbd48

[PATCH 1/2] tests: fix logic bug in init.sh

[Eric Blake]

If the shell test loop first finds a marginal then a good shell, the
variable $gl_set_x_corrupts_stderr is still set to true and needlessly
drops $VERBOSE logging.

* tests/init.sh: (gl_set_x_corrupts_stderr_): Clear for successful
shell.

Signed-off-by: Eric Blake 
---
 ChangeLog |6 ++
 tests/init.sh |5 -
 2 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index b7be3f3..2047b73 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2011-05-24  Eric Blake  
+
+   tests: fix logic bug in init.sh
+   * tests/init.sh: (gl_set_x_corrupts_stderr_): Clear for successful
+   shell.
+
 2011-05-24  Jim Meyering  

opendir-safer.c: don't clobber errno; don't close negative FD
diff --git a/tests/init.sh b/tests/init.sh
index 71c6516..294dcdd 100644
--- a/tests/init.sh
+++ b/tests/init.sh
@@ -167,7 +167,10 @@ else
 st_=$?

 # $re_shell_ works just fine.  Use it.
-test $st_ = 10 && break
+if test $st_ = 10; then
+  gl_set_x_corrupts_stderr_=false
+  break
+fi

 # If this is our first marginally acceptable shell, remember it.
 if test "$st_:$marginal_" = 9: ; then
-- 
1.7.4.4

[PATCH 2/2] perror: avoid spurious test failure on HP-UX

[Eric Blake]

The next-to-last command in this test has non-zero status.  Even
though 'exit 0' is supposed to ignore prior status, HP-UX /bin/sh
favors the prior status if an exit trap is installed.

* tests/test-perror.sh: Use Exit to avoid wrong exit status.

Signed-off-by: Eric Blake 
---
 ChangeLog|3 +++
 tests/test-perror.sh |2 +-
 2 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 2047b73..ecf18a1 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,8 @@
 2011-05-24  Eric Blake  

+   perror: avoid spurious test failure on HP-UX
+   * tests/test-perror.sh: Use Exit to avoid wrong exit status.
+
tests: fix logic bug in init.sh
* tests/init.sh: (gl_set_x_corrupts_stderr_): Clear for successful
shell.
diff --git a/tests/test-perror.sh b/tests/test-perror.sh
index 28027ea..7274d32 100755
--- a/tests/test-perror.sh
+++ b/tests/test-perror.sh
@@ -21,4 +21,4 @@ diff t-perror2.tmp t-perror3.tmp || fail_ "prefix applied 
incorrectly"
 test-perror >out 2>/dev/null || fail_ "unexpected exit status"
 test -s out && fail_ "unexpected output"

-exit 0
+Exit 0
-- 
1.7.4.4

Re: new files imported without new modules added

[Eric Blake]

On 05/24/2011 10:18 AM, Eric Blake wrote:
> On 05/24/2011 10:12 AM, Sam Steingold wrote:
>> Hi,
>> I updated gnulib and found that it now wants to add these files to
>> CLISP:
>>
>> src/gllib/glthread/lock.c
>> src/gllib/glthread/lock.h
>> src/gllib/glthread/threadlib.c
>> src/gllib/strerror-impl.h
>> src/gllib/strerror_r.c
>> src/glm4/strerror_r.m4
>>
>> I did not import any new modules.
>> what has happened?
> 
> strerror and perror now depend on strerror_r, and strerror_r depends on
> locking; this was done in order to fix perror bugs (perror is not
> allowed to modify the strerror static buffer).

That said, I would really like to make strerror_r's use of locking
conditional on whether 'lock' is independently present.  That is, for a
single-threaded application, having strerror_r call strerror without
locking is just fine, so the mere use of just the 'strerror' module
should not drag in locking.  strerror_r only needs locking if it is used
in a multi-threaded application.

That means that strerror-impl.h, strerror_r.c, and strerror_r.m4 will
still be needed, but it would get rid of the three glthread files.

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



signature.asc
Description: OpenPGP digital signature

Re: new files imported without new modules added

[Eric Blake]

On 05/24/2011 10:12 AM, Sam Steingold wrote:
> Hi,
> I updated gnulib and found that it now wants to add these files to
> CLISP:
> 
> src/gllib/glthread/lock.c
> src/gllib/glthread/lock.h
> src/gllib/glthread/threadlib.c
> src/gllib/strerror-impl.h
> src/gllib/strerror_r.c
> src/glm4/strerror_r.m4
> 
> I did not import any new modules.
> what has happened?

strerror and perror now depend on strerror_r, and strerror_r depends on
locking; this was done in order to fix perror bugs (perror is not
allowed to modify the strerror static buffer).

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



signature.asc
Description: OpenPGP digital signature

new files imported without new modules added

[Sam Steingold]

Hi,
I updated gnulib and found that it now wants to add these files to
CLISP:

src/gllib/glthread/lock.c
src/gllib/glthread/lock.h
src/gllib/glthread/threadlib.c
src/gllib/strerror-impl.h
src/gllib/strerror_r.c
src/glm4/strerror_r.m4

I did not import any new modules.
what has happened?

-- 
Sam Steingold (http://sds.podval.org/) on CentOS release 5.6 (Final) X 
11.0.60900031
http://camera.org http://www.memritv.org http://mideasttruth.com
http://www.PetitionOnline.com/tap12009/ http://pmw.org.il http://dhimmi.com
Any programming language is at its best before it is implemented and used.

Re: [PATCH] opendir-safer.c: don't clobber errno; don't close negative FD

[Eric Blake]

On 05/24/2011 05:49 AM, Jim Meyering wrote:
> Hi Eric,
> 
> coverity reported on the potential for closing a negative file descriptor.
> Fixing it, I saw/fixed the errno-clobbering problem.
> 
> Any objection?

Looks good; please apply.

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



signature.asc
Description: OpenPGP digital signature

[PATCH] opendir-safer.c: don't clobber errno; don't close negative FD

[Jim Meyering]

Hi Eric,

coverity reported on the potential for closing a negative file descriptor.
Fixing it, I saw/fixed the errno-clobbering problem.

Any objection?

Here's the slightly more readable form of the patch,
ignoring the indentation change:

diff --git a/lib/opendir-safer.c b/lib/opendir-safer.c
index f1e5fb7..3726f88 100644
--- a/lib/opendir-safer.c
+++ b/lib/opendir-safer.c
@@ -50,10 +50,18 @@ opendir_safer (char const *name)
   int e;
 #if HAVE_FDOPENDIR || GNULIB_FDOPENDIR
   int f = dup_safer (fd);
+  if (f < 0)
+{
+  e = errno;
+  newdp = NULL;
+}
+  else
+{
   newdp = fdopendir (f);
   e = errno;
   if (! newdp)
 close (f);
+}
 #else /* !FDOPENDIR */
   newdp = opendir_safer (name);
   e = errno;


>From d94bbd1eb1fc483d72397ec5dd94f7e885e12440 Mon Sep 17 00:00:00 2001
From: Jim Meyering 
Date: Tue, 24 May 2011 13:44:41 +0200
Subject: [PATCH] opendir-safer.c: don't clobber errno; don't close negative
 FD

* lib/opendir-safer.c (opendir_safer):
[HAVE_FDOPENDIR || GNULIB_FDOPENDIR]: Don't close a negative
file descriptor, and more importantly, don't clobber the
offending errno value with EINVAL.  Before, upon failure
of dup_safer, we would pass the negative file descriptor to
fdopendir, which would clobber errno.
---
 ChangeLog   |   10 ++
 lib/opendir-safer.c |   16 
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 1ab8584..b7be3f3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2011-05-24  Jim Meyering  
+
+   opendir-safer.c: don't clobber errno; don't close negative FD
+   * lib/opendir-safer.c (opendir_safer):
+   [HAVE_FDOPENDIR || GNULIB_FDOPENDIR]: Don't close a negative
+   file descriptor, and more importantly, don't clobber the
+   offending errno value with EINVAL.  Before, upon failure
+   of dup_safer, we would pass the negative file descriptor to
+   fdopendir, which would clobber errno.
+
 2011-05-23  Bruno Haible  

idcache: Fix module description.
diff --git a/lib/opendir-safer.c b/lib/opendir-safer.c
index f1e5fb7..3726f88 100644
--- a/lib/opendir-safer.c
+++ b/lib/opendir-safer.c
@@ -50,10 +50,18 @@ opendir_safer (char const *name)
   int e;
 #if HAVE_FDOPENDIR || GNULIB_FDOPENDIR
   int f = dup_safer (fd);
-  newdp = fdopendir (f);
-  e = errno;
-  if (! newdp)
-close (f);
+  if (f < 0)
+{
+  e = errno;
+  newdp = NULL;
+}
+  else
+{
+  newdp = fdopendir (f);
+  e = errno;
+  if (! newdp)
+close (f);
+}
 #else /* !FDOPENDIR */
   newdp = opendir_safer (name);
   e = errno;
--
1.7.5.2.585.gfbd48

Re: Move sha1 to C?

[Bruno Haible]

Ralf Wildenhues wrote:
> It's even easier to read this way IMVHO (and just as portable):
> 
>   sed_extract_condition2='
>     /^ *'"$escaped_dep"' *\[\(.*\)\] *$/ s//\1/p
>   '

Well, "easier to read" means that you have understood this paragraph from
the 'sed' specification [1]

  If an RE is empty (that is, no pattern is specified) sed shall behave
  as if the last RE used in the last command applied (either as an
  address or as part of a substitute command) was specified.

or from the GNU sed documentation [2]

  The empty regular expression ‘//’ repeats the last regular expression
  match (the same holds if the empty regular expression is passed to the
  s command). Note that modifiers to regular expressions are evaluated
  when the regular expression is compiled, thus it is invalid to specify
  them together with the empty regular expression.

It counts as an advanced feature of 'sed' for me.

Bruno

[1] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html
[2] http://www.gnu.org/software/sed/manual/html_node/Addresses.html