new files imported without new modules added

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: new files imported without new modules added

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 674 bytes --]

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   eblake@redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: new files imported without new modules added

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1269 bytes --]

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   eblake@redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: new files imported without new modules added

[Sam Steingold]

> * Eric Blake <roynxr@erqung.pbz> [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: new files imported without new modules added

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1467 bytes --]

On 05/24/2011 10:27 AM, Sam Steingold wrote:
>> * Eric Blake <roynxr@erqung.pbz> [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   eblake@redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: new files imported without new modules added

[Sam Steingold]

> * Eric Blake <roynxr@erqung.pbz> [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

[Sam Steingold]

> * Eric Blake <roynxr@erqung.pbz> [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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1527 bytes --]

On 05/24/2011 10:45 AM, Sam Steingold wrote:
>> * Eric Blake <roynxr@erqung.pbz> [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   eblake@redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: new files imported without new modules added

[Sam Steingold]

> * Eric Blake <roynxr@erqung.pbz> [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.

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2092 bytes --]

On 05/24/2011 12:06 PM, Sam Steingold wrote:
>> * Eric Blake <roynxr@erqung.pbz> [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   eblake@redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

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

[Simon Josefsson]

Eric Blake <eblake@redhat.com> writes:

> On 05/24/2011 12:06 PM, Sam Steingold wrote:
>>> * Eric Blake <roynxr@erqung.pbz> [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

Re: strerror vs. threads

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1254 bytes --]

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   eblake@redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: new files imported without new modules added

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1689 bytes --]

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   eblake@redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

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

[Sam Steingold]

> * Eric Blake <roynxr@erqung.pbz> [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: strerror vs. threads [was: new files imported without new modules added]

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1253 bytes --]

On 05/24/2011 01:27 PM, Sam Steingold wrote:
>> * Eric Blake <roynxr@erqung.pbz> [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   eblake@redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

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.

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
  <http://lists.gnu.org/archive/html/bug-gnulib/2010-09/msg00410.html>

> 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 <http://en.wikipedia.org/wiki/Georges_Darboy>

Re: strerror vs. threads

[Simon Josefsson]

Eric Blake <eblake@redhat.com> 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

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 <http://en.wikipedia.org/wiki/Georges_Darboy>

[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 <eblake@redhat.com>
---

> 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  <eblake@redhat.com>

+	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

[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 <eblake@redhat.com>
---

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  <eblake@redhat.com>

+	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..0000000
--- 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 <http://www.gnu.org/licenses/>.  */
-
-#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 0000000..5fcbe1f
--- /dev/null
+++ b/lib/strerror-override.c
@@ -0,0 +1,347 @@
+/* strerror-override.c --- POSIX compatible system error routine
+
+   Copyright (C) 2010-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 <http://www.gnu.org/licenses/>.  */
+
+/* Written by Bruno Haible <bruno@clisp.org>, 2010.  */
+
+#include "strerror-override.h"
+
+#include <errno.h>
+
+#if GNULIB_defined_ESOCK /* native Windows platforms */
+# if HAVE_WINSOCK2_H
+#  include <winsock2.h>
+# endif
+#endif
+
+/* This undefine allows testing with gl_cv_header_errno_h_complete=no on
+   a system that otherwise has a complete errno.h.  */
+#undef strerror_override
+
+/* If ERRNUM maps to an errno value defined by gnulib, return a string
+   describing the error.  Otherwise return NULL.  */
+const char *
+strerror_override (int errnum)
+{
+  const char *msg = NULL;
+
+#if GNULIB_defined_ETXTBSY \
+    || GNULIB_defined_ESOCK \
+    || GNULIB_defined_ENOMSG \
+    || GNULIB_defined_EIDRM \
+    || GNULIB_defined_ENOLINK \
+    || GNULIB_defined_EPROTO \
+    || GNULIB_defined_EMULTIHOP \
+    || GNULIB_defined_EBADMSG \
+    || GNULIB_defined_EOVERFLOW \
+    || GNULIB_defined_ENOTSUP \
+    || GNULIB_defined_ESTALE \
+    || GNULIB_defined_EDQUOT \
+    || GNULIB_defined_ECANCELED
+  /* These error messages are taken from glibc/sysdeps/gnu/errlist.c.  */
+  switch (errnum)
+    {
+# if GNULIB_defined_ETXTBSY
+    case ETXTBSY:
+      msg = "Text file busy";
+      break;
+# endif
+
+# if GNULIB_defined_ESOCK /* native Windows platforms */
+      /* EWOULDBLOCK is the same as EAGAIN.  */
+    case EINPROGRESS:
+      msg = "Operation now in progress";
+      break;
+    case EALREADY:
+      msg = "Operation already in progress";
+      break;
+    case ENOTSOCK:
+      msg = "Socket operation on non-socket";
+      break;
+    case EDESTADDRREQ:
+      msg = "Destination address required";
+      break;
+    case EMSGSIZE:
+      msg = "Message too long";
+      break;
+    case EPROTOTYPE:
+      msg = "Protocol wrong type for socket";
+      break;
+    case ENOPROTOOPT:
+      msg = "Protocol not available";
+      break;
+    case EPROTONOSUPPORT:
+      msg = "Protocol not supported";
+      break;
+    case ESOCKTNOSUPPORT:
+      msg = "Socket type not supported";
+      break;
+    case EOPNOTSUPP:
+      msg = "Operation not supported";
+      break;
+    case EPFNOSUPPORT:
+      msg = "Protocol family not supported";
+      break;
+    case EAFNOSUPPORT:
+      msg = "Address family not supported by protocol";
+      break;
+    case EADDRINUSE:
+      msg = "Address already in use";
+      break;
+    case EADDRNOTAVAIL:
+      msg = "Cannot assign requested address";
+      break;
+    case ENETDOWN:
+      msg = "Network is down";
+      break;
+    case ENETUNREACH:
+      msg = "Network is unreachable";
+      break;
+    case ENETRESET:
+      msg = "Network dropped connection on reset";
+      break;
+    case ECONNABORTED:
+      msg = "Software caused connection abort";
+      break;
+    case ECONNRESET:
+      msg = "Connection reset by peer";
+      break;
+    case ENOBUFS:
+      msg = "No buffer space available";
+      break;
+    case EISCONN:
+      msg = "Transport endpoint is already connected";
+      break;
+    case ENOTCONN:
+      msg = "Transport endpoint is not connected";
+      break;
+    case ESHUTDOWN:
+      msg = "Cannot send after transport endpoint shutdown";
+      break;
+    case ETOOMANYREFS:
+      msg = "Too many references: cannot splice";
+      break;
+    case ETIMEDOUT:
+      msg = "Connection timed out";
+      break;
+    case ECONNREFUSED:
+      msg = "Connection refused";
+      break;
+    case ELOOP:
+      msg = "Too many levels of symbolic links";
+      break;
+    case EHOSTDOWN:
+      msg = "Host is down";
+      break;
+    case EHOSTUNREACH:
+      msg = "No route to host";
+      break;
+    case EPROCLIM:
+      msg = "Too many processes";
+      break;
+    case EUSERS:
+      msg = "Too many users";
+      break;
+    case EDQUOT:
+      msg = "Disk quota exceeded";
+      break;
+    case ESTALE:
+      msg = "Stale NFS file handle";
+      break;
+    case EREMOTE:
+      msg = "Object is remote";
+      break;
+#  if HAVE_WINSOCK2_H
+      /* WSA_INVALID_HANDLE maps to EBADF */
+      /* WSA_NOT_ENOUGH_MEMORY maps to ENOMEM */
+      /* WSA_INVALID_PARAMETER maps to EINVAL */
+    case WSA_OPERATION_ABORTED:
+      msg = "Overlapped operation aborted";
+      break;
+    case WSA_IO_INCOMPLETE:
+      msg = "Overlapped I/O event object not in signaled state";
+      break;
+    case WSA_IO_PENDING:
+      msg = "Overlapped operations will complete later";
+      break;
+      /* WSAEINTR maps to EINTR */
+      /* WSAEBADF maps to EBADF */
+      /* WSAEACCES maps to EACCES */
+      /* WSAEFAULT maps to EFAULT */
+      /* WSAEINVAL maps to EINVAL */
+      /* WSAEMFILE maps to EMFILE */
+      /* WSAEWOULDBLOCK maps to EWOULDBLOCK */
+      /* WSAEINPROGRESS is EINPROGRESS */
+      /* WSAEALREADY is EALREADY */
+      /* WSAENOTSOCK is ENOTSOCK */
+      /* WSAEDESTADDRREQ is EDESTADDRREQ */
+      /* WSAEMSGSIZE is EMSGSIZE */
+      /* WSAEPROTOTYPE is EPROTOTYPE */
+      /* WSAENOPROTOOPT is ENOPROTOOPT */
+      /* WSAEPROTONOSUPPORT is EPROTONOSUPPORT */
+      /* WSAESOCKTNOSUPPORT is ESOCKTNOSUPPORT */
+      /* WSAEOPNOTSUPP is EOPNOTSUPP */
+      /* WSAEPFNOSUPPORT is EPFNOSUPPORT */
+      /* WSAEAFNOSUPPORT is EAFNOSUPPORT */
+      /* WSAEADDRINUSE is EADDRINUSE */
+      /* WSAEADDRNOTAVAIL is EADDRNOTAVAIL */
+      /* WSAENETDOWN is ENETDOWN */
+      /* WSAENETUNREACH is ENETUNREACH */
+      /* WSAENETRESET is ENETRESET */
+      /* WSAECONNABORTED is ECONNABORTED */
+      /* WSAECONNRESET is ECONNRESET */
+      /* WSAENOBUFS is ENOBUFS */
+      /* WSAEISCONN is EISCONN */
+      /* WSAENOTCONN is ENOTCONN */
+      /* WSAESHUTDOWN is ESHUTDOWN */
+      /* WSAETOOMANYREFS is ETOOMANYREFS */
+      /* WSAETIMEDOUT is ETIMEDOUT */
+      /* WSAECONNREFUSED is ECONNREFUSED */
+      /* WSAELOOP is ELOOP */
+      /* WSAENAMETOOLONG maps to ENAMETOOLONG */
+      /* WSAEHOSTDOWN is EHOSTDOWN */
+      /* WSAEHOSTUNREACH is EHOSTUNREACH */
+      /* WSAENOTEMPTY maps to ENOTEMPTY */
+      /* WSAEPROCLIM is EPROCLIM */
+      /* WSAEUSERS is EUSERS */
+      /* WSAEDQUOT is EDQUOT */
+      /* WSAESTALE is ESTALE */
+      /* WSAEREMOTE is EREMOTE */
+    case WSASYSNOTREADY:
+      msg = "Network subsystem is unavailable";
+      break;
+    case WSAVERNOTSUPPORTED:
+      msg = "Winsock.dll version out of range";
+      break;
+    case WSANOTINITIALISED:
+      msg = "Successful WSAStartup not yet performed";
+      break;
+    case WSAEDISCON:
+      msg = "Graceful shutdown in progress";
+      break;
+    case WSAENOMORE: case WSA_E_NO_MORE:
+      msg = "No more results";
+      break;
+    case WSAECANCELLED: case WSA_E_CANCELLED:
+      msg = "Call was canceled";
+      break;
+    case WSAEINVALIDPROCTABLE:
+      msg = "Procedure call table is invalid";
+      break;
+    case WSAEINVALIDPROVIDER:
+      msg = "Service provider is invalid";
+      break;
+    case WSAEPROVIDERFAILEDINIT:
+      msg = "Service provider failed to initialize";
+      break;
+    case WSASYSCALLFAILURE:
+      msg = "System call failure";
+      break;
+    case WSASERVICE_NOT_FOUND:
+      msg = "Service not found";
+      break;
+    case WSATYPE_NOT_FOUND:
+      msg = "Class type not found";
+      break;
+    case WSAEREFUSED:
+      msg = "Database query was refused";
+      break;
+    case WSAHOST_NOT_FOUND:
+      msg = "Host not found";
+      break;
+    case WSATRY_AGAIN:
+      msg = "Nonauthoritative host not found";
+      break;
+    case WSANO_RECOVERY:
+      msg = "Nonrecoverable error";
+      break;
+    case WSANO_DATA:
+      msg = "Valid name, no data record of requested type";
+      break;
+      /* WSA_QOS_* omitted */
+#  endif
+# endif
+
+# if GNULIB_defined_ENOMSG
+    case ENOMSG:
+      msg = "No message of desired type";
+      break;
+# endif
+
+# if GNULIB_defined_EIDRM
+    case EIDRM:
+      msg = "Identifier removed";
+      break;
+# endif
+
+# if GNULIB_defined_ENOLINK
+    case ENOLINK:
+      msg = "Link has been severed";
+      break;
+# endif
+
+# if GNULIB_defined_EPROTO
+    case EPROTO:
+      msg = "Protocol error";
+      break;
+# endif
+
+# if GNULIB_defined_EMULTIHOP
+    case EMULTIHOP:
+      msg = "Multihop attempted";
+      break;
+# endif
+
+# if GNULIB_defined_EBADMSG
+    case EBADMSG:
+      msg = "Bad message";
+      break;
+# endif
+
+# if GNULIB_defined_EOVERFLOW
+    case EOVERFLOW:
+      msg = "Value too large for defined data type";
+      break;
+# endif
+
+# if GNULIB_defined_ENOTSUP
+    case ENOTSUP:
+      msg = "Not supported";
+      break;
+# endif
+
+# if GNULIB_defined_ESTALE
+    case ESTALE:
+      msg = "Stale NFS file handle";
+      break;
+# endif
+
+# if GNULIB_defined_EDQUOT
+    case EDQUOT:
+      msg = "Disk quota exceeded";
+      break;
+# endif
+
+# if GNULIB_defined_ECANCELED
+    case ECANCELED:
+      msg = "Operation canceled";
+      break;
+# endif
+    }
+#endif
+
+  return msg;
+}
diff --git a/lib/strerror-override.h b/lib/strerror-override.h
new file mode 100644
index 0000000..0718b53
--- /dev/null
+++ b/lib/strerror-override.h
@@ -0,0 +1,48 @@
+/* strerror-override.h --- POSIX compatible system error routine
+
+   Copyright (C) 2010-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 <http://www.gnu.org/licenses/>.  */
+
+#ifndef _GL_STRERROR_OVERRIDE_H
+# define _GL_STRERROR_OVERRIDE_H
+
+# include <string.h>
+
+/* Reasonable buffer size that should never trigger ERANGE; if this
+   proves too small, we intentionally abort(), to remind us to fix
+   this value.  */
+# define STACKBUF_LEN 256
+
+/* If ERRNUM maps to an errno value defined by gnulib, return a string
+   describing the error.  Otherwise return NULL.  */
+# if GNULIB_defined_ETXTBSY \
+    || GNULIB_defined_ESOCK \
+    || GNULIB_defined_ENOMSG \
+    || GNULIB_defined_EIDRM \
+    || GNULIB_defined_ENOLINK \
+    || GNULIB_defined_EPROTO \
+    || GNULIB_defined_EMULTIHOP \
+    || GNULIB_defined_EBADMSG \
+    || GNULIB_defined_EOVERFLOW \
+    || GNULIB_defined_ENOTSUP \
+    || GNULIB_defined_ESTALE \
+    || GNULIB_defined_EDQUOT \
+    || GNULIB_defined_ECANCELED
+extern const char *strerror_override (int errnum);
+# else
+#  define strerror_override(ignored) NULL
+# endif
+
+#endif /* _GL_STRERROR_OVERRIDE_H */
diff --git a/lib/strerror.c b/lib/strerror.c
index 9d1f165..8c41179 100644
--- a/lib/strerror.c
+++ b/lib/strerror.c
@@ -23,11 +23,45 @@
 #include <errno.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>

 #include "intprops.h"
+#include "strerror-override.h"
 #include "verify.h"

 /* Use the system functions, not the gnulib overrides in this file.  */
 #undef sprintf

-#include "strerror-impl.h"
+char *
+strerror (int n)
+#undef strerror
+{
+  static char buf[STACKBUF_LEN];
+  size_t len;
+
+  /* Cast away const, due to the historical signature of strerror;
+     callers should not be modifying the string.  */
+  const char *msg = strerror_override (n);
+  if (msg)
+    return (char *) msg;
+
+  /* Our strerror_r implementation might use the system's strerror
+     buffer, so all other clients of strerror have to see the error
+     copied into a buffer that we manage.  */
+  msg = strerror (n);
+  if (!msg || !*msg)
+    {
+      static char const fmt[] = "Unknown error %d";
+      verify (sizeof buf >= sizeof (fmt) + INT_STRLEN_BOUND (n));
+      sprintf (buf, fmt, n);
+      errno = EINVAL;
+      return buf;
+    }
+
+  /* Fix STACKBUF_LEN if this ever aborts.  */
+  len = strlen (msg);
+  if (sizeof buf <= len)
+    abort ();
+
+  return memcpy (buf, msg, len + 1);
+}
diff --git a/lib/strerror_r.c b/lib/strerror_r.c
index 494b1f0..462455d 100644
--- a/lib/strerror_r.c
+++ b/lib/strerror_r.c
@@ -28,16 +28,7 @@
 #include <errno.h>
 #include <stdio.h>

-#if GNULIB_defined_ESOCK /* native Windows platforms */
-# if HAVE_WINSOCK2_H
-#  include <winsock2.h>
-# 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
+#include "strerror-override.h"

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

@@ -136,316 +127,13 @@ strerror_r (int errnum, char *buf, size_t buflen)
     }
   *buf = '\0';

-#if GNULIB_defined_ETXTBSY \
-    || GNULIB_defined_ESOCK \
-    || GNULIB_defined_ENOMSG \
-    || GNULIB_defined_EIDRM \
-    || GNULIB_defined_ENOLINK \
-    || GNULIB_defined_EPROTO \
-    || GNULIB_defined_EMULTIHOP \
-    || GNULIB_defined_EBADMSG \
-    || GNULIB_defined_EOVERFLOW \
-    || GNULIB_defined_ENOTSUP \
-    || GNULIB_defined_ESTALE \
-    || GNULIB_defined_EDQUOT \
-    || GNULIB_defined_ECANCELED
+  /* Check for gnulib overrides.  */
   {
-    char const *msg = NULL;
-    /* These error messages are taken from glibc/sysdeps/gnu/errlist.c.  */
-    switch (errnum)
-      {
-# if GNULIB_defined_ETXTBSY
-      case ETXTBSY:
-        msg = "Text file busy";
-        break;
-# endif
-
-# if GNULIB_defined_ESOCK /* native Windows platforms */
-      /* EWOULDBLOCK is the same as EAGAIN.  */
-      case EINPROGRESS:
-        msg = "Operation now in progress";
-        break;
-      case EALREADY:
-        msg = "Operation already in progress";
-        break;
-      case ENOTSOCK:
-        msg = "Socket operation on non-socket";
-        break;
-      case EDESTADDRREQ:
-        msg = "Destination address required";
-        break;
-      case EMSGSIZE:
-        msg = "Message too long";
-        break;
-      case EPROTOTYPE:
-        msg = "Protocol wrong type for socket";
-        break;
-      case ENOPROTOOPT:
-        msg = "Protocol not available";
-        break;
-      case EPROTONOSUPPORT:
-        msg = "Protocol not supported";
-        break;
-      case ESOCKTNOSUPPORT:
-        msg = "Socket type not supported";
-        break;
-      case EOPNOTSUPP:
-        msg = "Operation not supported";
-        break;
-      case EPFNOSUPPORT:
-        msg = "Protocol family not supported";
-        break;
-      case EAFNOSUPPORT:
-        msg = "Address family not supported by protocol";
-        break;
-      case EADDRINUSE:
-        msg = "Address already in use";
-        break;
-      case EADDRNOTAVAIL:
-        msg = "Cannot assign requested address";
-        break;
-      case ENETDOWN:
-        msg = "Network is down";
-        break;
-      case ENETUNREACH:
-        msg = "Network is unreachable";
-        break;
-      case ENETRESET:
-        msg = "Network dropped connection on reset";
-        break;
-      case ECONNABORTED:
-        msg = "Software caused connection abort";
-        break;
-      case ECONNRESET:
-        msg = "Connection reset by peer";
-        break;
-      case ENOBUFS:
-        msg = "No buffer space available";
-        break;
-      case EISCONN:
-        msg = "Transport endpoint is already connected";
-        break;
-      case ENOTCONN:
-        msg = "Transport endpoint is not connected";
-        break;
-      case ESHUTDOWN:
-        msg = "Cannot send after transport endpoint shutdown";
-        break;
-      case ETOOMANYREFS:
-        msg = "Too many references: cannot splice";
-        break;
-      case ETIMEDOUT:
-        msg = "Connection timed out";
-        break;
-      case ECONNREFUSED:
-        msg = "Connection refused";
-        break;
-      case ELOOP:
-        msg = "Too many levels of symbolic links";
-        break;
-      case EHOSTDOWN:
-        msg = "Host is down";
-        break;
-      case EHOSTUNREACH:
-        msg = "No route to host";
-        break;
-      case EPROCLIM:
-        msg = "Too many processes";
-        break;
-      case EUSERS:
-        msg = "Too many users";
-        break;
-      case EDQUOT:
-        msg = "Disk quota exceeded";
-        break;
-      case ESTALE:
-        msg = "Stale NFS file handle";
-        break;
-      case EREMOTE:
-        msg = "Object is remote";
-        break;
-#  if HAVE_WINSOCK2_H
-      /* WSA_INVALID_HANDLE maps to EBADF */
-      /* WSA_NOT_ENOUGH_MEMORY maps to ENOMEM */
-      /* WSA_INVALID_PARAMETER maps to EINVAL */
-      case WSA_OPERATION_ABORTED:
-        msg = "Overlapped operation aborted";
-        break;
-      case WSA_IO_INCOMPLETE:
-        msg = "Overlapped I/O event object not in signaled state";
-        break;
-      case WSA_IO_PENDING:
-        msg = "Overlapped operations will complete later";
-        break;
-      /* WSAEINTR maps to EINTR */
-      /* WSAEBADF maps to EBADF */
-      /* WSAEACCES maps to EACCES */
-      /* WSAEFAULT maps to EFAULT */
-      /* WSAEINVAL maps to EINVAL */
-      /* WSAEMFILE maps to EMFILE */
-      /* WSAEWOULDBLOCK maps to EWOULDBLOCK */
-      /* WSAEINPROGRESS is EINPROGRESS */
-      /* WSAEALREADY is EALREADY */
-      /* WSAENOTSOCK is ENOTSOCK */
-      /* WSAEDESTADDRREQ is EDESTADDRREQ */
-      /* WSAEMSGSIZE is EMSGSIZE */
-      /* WSAEPROTOTYPE is EPROTOTYPE */
-      /* WSAENOPROTOOPT is ENOPROTOOPT */
-      /* WSAEPROTONOSUPPORT is EPROTONOSUPPORT */
-      /* WSAESOCKTNOSUPPORT is ESOCKTNOSUPPORT */
-      /* WSAEOPNOTSUPP is EOPNOTSUPP */
-      /* WSAEPFNOSUPPORT is EPFNOSUPPORT */
-      /* WSAEAFNOSUPPORT is EAFNOSUPPORT */
-      /* WSAEADDRINUSE is EADDRINUSE */
-      /* WSAEADDRNOTAVAIL is EADDRNOTAVAIL */
-      /* WSAENETDOWN is ENETDOWN */
-      /* WSAENETUNREACH is ENETUNREACH */
-      /* WSAENETRESET is ENETRESET */
-      /* WSAECONNABORTED is ECONNABORTED */
-      /* WSAECONNRESET is ECONNRESET */
-      /* WSAENOBUFS is ENOBUFS */
-      /* WSAEISCONN is EISCONN */
-      /* WSAENOTCONN is ENOTCONN */
-      /* WSAESHUTDOWN is ESHUTDOWN */
-      /* WSAETOOMANYREFS is ETOOMANYREFS */
-      /* WSAETIMEDOUT is ETIMEDOUT */
-      /* WSAECONNREFUSED is ECONNREFUSED */
-      /* WSAELOOP is ELOOP */
-      /* WSAENAMETOOLONG maps to ENAMETOOLONG */
-      /* WSAEHOSTDOWN is EHOSTDOWN */
-      /* WSAEHOSTUNREACH is EHOSTUNREACH */
-      /* WSAENOTEMPTY maps to ENOTEMPTY */
-      /* WSAEPROCLIM is EPROCLIM */
-      /* WSAEUSERS is EUSERS */
-      /* WSAEDQUOT is EDQUOT */
-      /* WSAESTALE is ESTALE */
-      /* WSAEREMOTE is EREMOTE */
-      case WSASYSNOTREADY:
-        msg = "Network subsystem is unavailable";
-        break;
-      case WSAVERNOTSUPPORTED:
-        msg = "Winsock.dll version out of range";
-        break;
-      case WSANOTINITIALISED:
-        msg = "Successful WSAStartup not yet performed";
-        break;
-      case WSAEDISCON:
-        msg = "Graceful shutdown in progress";
-        break;
-      case WSAENOMORE: case WSA_E_NO_MORE:
-        msg = "No more results";
-        break;
-      case WSAECANCELLED: case WSA_E_CANCELLED:
-        msg = "Call was canceled";
-        break;
-      case WSAEINVALIDPROCTABLE:
-        msg = "Procedure call table is invalid";
-        break;
-      case WSAEINVALIDPROVIDER:
-        msg = "Service provider is invalid";
-        break;
-      case WSAEPROVIDERFAILEDINIT:
-        msg = "Service provider failed to initialize";
-        break;
-      case WSASYSCALLFAILURE:
-        msg = "System call failure";
-        break;
-      case WSASERVICE_NOT_FOUND:
-        msg = "Service not found";
-        break;
-      case WSATYPE_NOT_FOUND:
-        msg = "Class type not found";
-        break;
-      case WSAEREFUSED:
-        msg = "Database query was refused";
-        break;
-      case WSAHOST_NOT_FOUND:
-        msg = "Host not found";
-        break;
-      case WSATRY_AGAIN:
-        msg = "Nonauthoritative host not found";
-        break;
-      case WSANO_RECOVERY:
-        msg = "Nonrecoverable error";
-        break;
-      case WSANO_DATA:
-        msg = "Valid name, no data record of requested type";
-        break;
-      /* WSA_QOS_* omitted */
-#  endif
-# endif
-
-# if GNULIB_defined_ENOMSG
-      case ENOMSG:
-        msg = "No message of desired type";
-        break;
-# endif
-
-# if GNULIB_defined_EIDRM
-      case EIDRM:
-        msg = "Identifier removed";
-        break;
-# endif
-
-# if GNULIB_defined_ENOLINK
-      case ENOLINK:
-        msg = "Link has been severed";
-        break;
-# endif
-
-# if GNULIB_defined_EPROTO
-      case EPROTO:
-        msg = "Protocol error";
-        break;
-# endif
-
-# if GNULIB_defined_EMULTIHOP
-      case EMULTIHOP:
-        msg = "Multihop attempted";
-        break;
-# endif
-
-# if GNULIB_defined_EBADMSG
-      case EBADMSG:
-        msg = "Bad message";
-        break;
-# endif
-
-# if GNULIB_defined_EOVERFLOW
-      case EOVERFLOW:
-        msg = "Value too large for defined data type";
-        break;
-# endif
-
-# if GNULIB_defined_ENOTSUP
-      case ENOTSUP:
-        msg = "Not supported";
-        break;
-# endif
-
-# if GNULIB_defined_ESTALE
-      case ESTALE:
-        msg = "Stale NFS file handle";
-        break;
-# endif
-
-# if GNULIB_defined_EDQUOT
-      case EDQUOT:
-        msg = "Disk quota exceeded";
-        break;
-# endif
-
-# if GNULIB_defined_ECANCELED
-      case ECANCELED:
-        msg = "Operation canceled";
-        break;
-# endif
-      }
+    char const *msg = strerror_override (errnum);

     if (msg)
       return safe_copy (buf, buflen, msg);
   }
-#endif

   {
     int ret;
diff --git a/modules/strerror b/modules/strerror
index 0e7a254..804df27 100644
--- a/modules/strerror
+++ b/modules/strerror
@@ -3,15 +3,14 @@ strerror() function: return string describing error code.

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

 Depends-on:
 string
-errno            [test $REPLACE_STRERROR = 1]
-intprops         [test $REPLACE_STRERROR = 1]
-verify           [test $REPLACE_STRERROR = 1]
-strerror_r-posix [test $REPLACE_STRERROR = 1]
+errno             [test $REPLACE_STRERROR = 1]
+intprops          [test $REPLACE_STRERROR = 1]
+strerror-override [test $REPLACE_STRERROR = 1]
+verify            [test $REPLACE_STRERROR = 1]

 configure.ac:
 gl_FUNC_STRERROR
diff --git a/modules/strerror-override b/modules/strerror-override
new file mode 100644
index 0000000..a31e8a1
--- /dev/null
+++ b/modules/strerror-override
@@ -0,0 +1,26 @@
+Description:
+strerror_override() function: provide strings for gnulib-specific errno values
+
+Files:
+lib/strerror-override.h
+lib/strerror-override.c
+
+Depends-on:
+errno
+
+configure.ac:
+AC_REQUIRE([gl_HEADER_ERRNO_H])
+if test -n "$ERRNO_H"; then
+  AC_LIBOBJ([strerror-override])
+fi
+
+Makefile.am:
+
+Include:
+"strerror-override.h"
+
+License:
+LGPLv2+
+
+Maintainer:
+all
diff --git a/modules/strerror_r-posix b/modules/strerror_r-posix
index a36f2e6..3c80d79 100644
--- a/modules/strerror_r-posix
+++ b/modules/strerror_r-posix
@@ -10,6 +10,7 @@ string
 extensions
 errno           [test $HAVE_DECL_STRERROR_R = 0 || test $REPLACE_STRERROR_R = 1]
 lock            [test $HAVE_DECL_STRERROR_R = 0 || test $REPLACE_STRERROR_R = 1]
+strerror-override [test $HAVE_DECL_STRERROR_R = 0 || test $REPLACE_STRERROR_R = 1]

 configure.ac:
 gl_FUNC_STRERROR_R
-- 
1.7.4.4

Re: new files imported without new modules added

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1075 bytes --]

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   eblake@redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: strerror vs. threads

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2027 bytes --]

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   eblake@redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: strerror vs. threads

[Simon Josefsson]

Eric Blake <eblake@redhat.com> writes:

> 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.

Yes.  What we'd really like is a dependency scheme that can express
something like "strerror_r-gnu | strerror_r-posix"...

>> 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).

Ah.  That's bad, and should definitely be replaced..

> 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.

..although if it comes with the portability hurdles of threads, I'm not
convinced it will be "easier" for everyone in the long run.

/Simon

Re: strerror vs. threads

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2785 bytes --]

On 05/25/2011 02:59 AM, Simon Josefsson wrote:
>> 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.
> 
> Yes.  What we'd really like is a dependency scheme that can express
> something like "strerror_r-gnu | strerror_r-posix"...

But that _still_ leaves us with the issue of how do you know which
signature of strerror_r you are calling?  Remember, glibc
strerror_r(EACCES,buf,len) leaves buf untouched - you are expected to
use the returned pointer; but XPG strerror_r(EACCES,buf,len) returns an
int which is almost guaranteed to be fatal if you dereference it as a char*.

If glibc were to change GNU strerror_r to _always_ populate buf, in
addition to the return string, then you could blindly call either
variant of strerror_r and just ignore the return value; the only
remaining problem is making sure you provide a large enough buffer for
any resulting string, since you no longer have error checking to tell if
buf got truncated.  I guess it's worth a shot asking Uli if he'll make
that change in glibc (since he already changed __xpg_strerror_r to
populate buf).

>> 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.
> 
> ..although if it comes with the portability hurdles of threads, I'm not
> convinced it will be "easier" for everyone in the long run.

Which somewhat goes back to my question of whether using the strerror_r
in a single-thread program should drag in the lock module, or if it
acceptable to conditionally compile in the lock code in strerror_r.c if
lock is present but otherwise skip locking, under the assumption that
any multi-threaded program is explicitly using locks in addition to
strerror_r.

Or maybe make a 'strerror_r-posix-simple' that is not thread-safe, which
does not depend on 'lock', and 'strerror_r-posix' depends on
'strerror_r-posix-simple' and on 'lock', so that the default module name
is thread-safe, and you have to ask for the longer module name if you
explicitly don't care about locking but want to use the reentrant
interface.  Then again, if you are single-threaded, then 'strerror' is
easier to use than 'strerror_r'.  So I've somewhat convinced myself that
things are fine as they are - strerror_r-posix should drag in 'lock',
and we don't need a single-threaded variant there.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

coping with conflicting strerror_r signatures [was: strerror vs. threads]

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2130 bytes --]

On 05/25/2011 08:32 AM, Eric Blake wrote:
> If glibc were to change GNU strerror_r to _always_ populate buf, in
> addition to the return string, then you could blindly call either
> variant of strerror_r and just ignore the return value; the only
> remaining problem is making sure you provide a large enough buffer for
> any resulting string, since you no longer have error checking to tell if
> buf got truncated.  I guess it's worth a shot asking Uli if he'll make
> that change in glibc (since he already changed __xpg_strerror_r to
> populate buf).

http://sourceware.org/bugzilla/show_bug.cgi?id=12805

If Uli agrees to fix it in glibc.git, then we are that much closer to
having a strerror_r-gnu that guarantees that buf is always populated (by
replacing strerror_r on older glibc), at which point it no longer
matters which return type is in effect.  That gets us closer to the idea
of requiring that a strerror_r be present, but we don't care if it is
strerror_r-posix or strerror_r-gnu.  At which point, I could see doing
the following:

perror depends on strerror_r-posix, but ignores the return type, so that
it also works with strerror_r-gnu (more generally, any gnulib code that
depends on error strings would be written to depend on strerror_r-posix,
but to also work if strerror_r-gnu is in use)

strerror_r-gnu depends on and overrides strerror_r-posix to guarantee
the GNU return type; without it, you are left with the POSIX signature.
 While gnulib code can't depend on the return type, your application
can, because you know which of the two modules you imported.
Additionally, the strerror_r-gnu module would expose __xpg_strerror_r on
all platforms (not just glibc and cygwin), to make it possible to still
get at the POSIX signature, although we might go with the nicer name
xpg_strerror_r (no leading underscores).

test-strerror_r checks C witness macros to see which signature is in
effect, in order to determine which aspects of the return type to check.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [PATCH 1/2] perror: call strerror_r directly

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 605 bytes --]

On 05/24/2011 03:41 PM, Eric Blake wrote:
> 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 <eblake@redhat.com>
> ---

No comments, so I've pushed this.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

[PATCHv2] strerror: drop strerror_r dependency

[Eric Blake]

Since the errno module is responsible for introducing replacement
errno values, it should also be responsible for translating those
new values into strings.  And by moving the replacements into a
file managed by the errno, we can then break the dependency between
strerror and strerror_r, so that strerror no longer drags in
multi-threading modules required by 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

* 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/errno (Files): Add new files.
* m4/errno_h.m4 (gl_HEADER_ERRNO_H): Compile new file.
* modules/strerror (Files): Drop unused file.
(Depends-on): Drop strerror_r-posix.
* MODULES.html.sh: Document strerror_r-posix.
Requested by Sam Steingold.

Signed-off-by: Eric Blake <eblake@redhat.com>
---

No comments since v1, so I'm going ahead and pushing this; v2
addressed my earlier concern:

> 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         |    1 +
 lib/strerror-impl.h     |   42 ------
 lib/strerror-override.c |  347 +++++++++++++++++++++++++++++++++++++++++++++++
 lib/strerror-override.h |   48 +++++++
 lib/strerror.c          |   36 +++++-
 lib/strerror_r.c        |  318 +------------------------------------------
 m4/errno_h.m4           |    3 +-
 modules/errno           |    2 +
 modules/strerror        |    2 -
 10 files changed, 451 insertions(+), 361 deletions(-)
 delete mode 100644 lib/strerror-impl.h
 create mode 100644 lib/strerror-override.c
 create mode 100644 lib/strerror-override.h

diff --git a/ChangeLog b/ChangeLog
index 0685dcc..920d8ca 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,18 @@
 2011-06-01  Eric Blake  <eblake@redhat.com>

+	strerror: drop strerror_r dependency
+	* 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/errno (Files): Add new files.
+	* m4/errno_h.m4 (gl_HEADER_ERRNO_H): Compile new file.
+	* modules/strerror (Files): Drop unused file.
+	(Depends-on): Drop strerror_r-posix.
+	* MODULES.html.sh: Document 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..f929ecf 100755
--- a/MODULES.html.sh
+++ b/MODULES.html.sh
@@ -1789,6 +1789,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..0000000
--- 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 <http://www.gnu.org/licenses/>.  */
-
-#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 0000000..5fcbe1f
--- /dev/null
+++ b/lib/strerror-override.c
@@ -0,0 +1,347 @@
+/* strerror-override.c --- POSIX compatible system error routine
+
+   Copyright (C) 2010-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 <http://www.gnu.org/licenses/>.  */
+
+/* Written by Bruno Haible <bruno@clisp.org>, 2010.  */
+
+#include "strerror-override.h"
+
+#include <errno.h>
+
+#if GNULIB_defined_ESOCK /* native Windows platforms */
+# if HAVE_WINSOCK2_H
+#  include <winsock2.h>
+# endif
+#endif
+
+/* This undefine allows testing with gl_cv_header_errno_h_complete=no on
+   a system that otherwise has a complete errno.h.  */
+#undef strerror_override
+
+/* If ERRNUM maps to an errno value defined by gnulib, return a string
+   describing the error.  Otherwise return NULL.  */
+const char *
+strerror_override (int errnum)
+{
+  const char *msg = NULL;
+
+#if GNULIB_defined_ETXTBSY \
+    || GNULIB_defined_ESOCK \
+    || GNULIB_defined_ENOMSG \
+    || GNULIB_defined_EIDRM \
+    || GNULIB_defined_ENOLINK \
+    || GNULIB_defined_EPROTO \
+    || GNULIB_defined_EMULTIHOP \
+    || GNULIB_defined_EBADMSG \
+    || GNULIB_defined_EOVERFLOW \
+    || GNULIB_defined_ENOTSUP \
+    || GNULIB_defined_ESTALE \
+    || GNULIB_defined_EDQUOT \
+    || GNULIB_defined_ECANCELED
+  /* These error messages are taken from glibc/sysdeps/gnu/errlist.c.  */
+  switch (errnum)
+    {
+# if GNULIB_defined_ETXTBSY
+    case ETXTBSY:
+      msg = "Text file busy";
+      break;
+# endif
+
+# if GNULIB_defined_ESOCK /* native Windows platforms */
+      /* EWOULDBLOCK is the same as EAGAIN.  */
+    case EINPROGRESS:
+      msg = "Operation now in progress";
+      break;
+    case EALREADY:
+      msg = "Operation already in progress";
+      break;
+    case ENOTSOCK:
+      msg = "Socket operation on non-socket";
+      break;
+    case EDESTADDRREQ:
+      msg = "Destination address required";
+      break;
+    case EMSGSIZE:
+      msg = "Message too long";
+      break;
+    case EPROTOTYPE:
+      msg = "Protocol wrong type for socket";
+      break;
+    case ENOPROTOOPT:
+      msg = "Protocol not available";
+      break;
+    case EPROTONOSUPPORT:
+      msg = "Protocol not supported";
+      break;
+    case ESOCKTNOSUPPORT:
+      msg = "Socket type not supported";
+      break;
+    case EOPNOTSUPP:
+      msg = "Operation not supported";
+      break;
+    case EPFNOSUPPORT:
+      msg = "Protocol family not supported";
+      break;
+    case EAFNOSUPPORT:
+      msg = "Address family not supported by protocol";
+      break;
+    case EADDRINUSE:
+      msg = "Address already in use";
+      break;
+    case EADDRNOTAVAIL:
+      msg = "Cannot assign requested address";
+      break;
+    case ENETDOWN:
+      msg = "Network is down";
+      break;
+    case ENETUNREACH:
+      msg = "Network is unreachable";
+      break;
+    case ENETRESET:
+      msg = "Network dropped connection on reset";
+      break;
+    case ECONNABORTED:
+      msg = "Software caused connection abort";
+      break;
+    case ECONNRESET:
+      msg = "Connection reset by peer";
+      break;
+    case ENOBUFS:
+      msg = "No buffer space available";
+      break;
+    case EISCONN:
+      msg = "Transport endpoint is already connected";
+      break;
+    case ENOTCONN:
+      msg = "Transport endpoint is not connected";
+      break;
+    case ESHUTDOWN:
+      msg = "Cannot send after transport endpoint shutdown";
+      break;
+    case ETOOMANYREFS:
+      msg = "Too many references: cannot splice";
+      break;
+    case ETIMEDOUT:
+      msg = "Connection timed out";
+      break;
+    case ECONNREFUSED:
+      msg = "Connection refused";
+      break;
+    case ELOOP:
+      msg = "Too many levels of symbolic links";
+      break;
+    case EHOSTDOWN:
+      msg = "Host is down";
+      break;
+    case EHOSTUNREACH:
+      msg = "No route to host";
+      break;
+    case EPROCLIM:
+      msg = "Too many processes";
+      break;
+    case EUSERS:
+      msg = "Too many users";
+      break;
+    case EDQUOT:
+      msg = "Disk quota exceeded";
+      break;
+    case ESTALE:
+      msg = "Stale NFS file handle";
+      break;
+    case EREMOTE:
+      msg = "Object is remote";
+      break;
+#  if HAVE_WINSOCK2_H
+      /* WSA_INVALID_HANDLE maps to EBADF */
+      /* WSA_NOT_ENOUGH_MEMORY maps to ENOMEM */
+      /* WSA_INVALID_PARAMETER maps to EINVAL */
+    case WSA_OPERATION_ABORTED:
+      msg = "Overlapped operation aborted";
+      break;
+    case WSA_IO_INCOMPLETE:
+      msg = "Overlapped I/O event object not in signaled state";
+      break;
+    case WSA_IO_PENDING:
+      msg = "Overlapped operations will complete later";
+      break;
+      /* WSAEINTR maps to EINTR */
+      /* WSAEBADF maps to EBADF */
+      /* WSAEACCES maps to EACCES */
+      /* WSAEFAULT maps to EFAULT */
+      /* WSAEINVAL maps to EINVAL */
+      /* WSAEMFILE maps to EMFILE */
+      /* WSAEWOULDBLOCK maps to EWOULDBLOCK */
+      /* WSAEINPROGRESS is EINPROGRESS */
+      /* WSAEALREADY is EALREADY */
+      /* WSAENOTSOCK is ENOTSOCK */
+      /* WSAEDESTADDRREQ is EDESTADDRREQ */
+      /* WSAEMSGSIZE is EMSGSIZE */
+      /* WSAEPROTOTYPE is EPROTOTYPE */
+      /* WSAENOPROTOOPT is ENOPROTOOPT */
+      /* WSAEPROTONOSUPPORT is EPROTONOSUPPORT */
+      /* WSAESOCKTNOSUPPORT is ESOCKTNOSUPPORT */
+      /* WSAEOPNOTSUPP is EOPNOTSUPP */
+      /* WSAEPFNOSUPPORT is EPFNOSUPPORT */
+      /* WSAEAFNOSUPPORT is EAFNOSUPPORT */
+      /* WSAEADDRINUSE is EADDRINUSE */
+      /* WSAEADDRNOTAVAIL is EADDRNOTAVAIL */
+      /* WSAENETDOWN is ENETDOWN */
+      /* WSAENETUNREACH is ENETUNREACH */
+      /* WSAENETRESET is ENETRESET */
+      /* WSAECONNABORTED is ECONNABORTED */
+      /* WSAECONNRESET is ECONNRESET */
+      /* WSAENOBUFS is ENOBUFS */
+      /* WSAEISCONN is EISCONN */
+      /* WSAENOTCONN is ENOTCONN */
+      /* WSAESHUTDOWN is ESHUTDOWN */
+      /* WSAETOOMANYREFS is ETOOMANYREFS */
+      /* WSAETIMEDOUT is ETIMEDOUT */
+      /* WSAECONNREFUSED is ECONNREFUSED */
+      /* WSAELOOP is ELOOP */
+      /* WSAENAMETOOLONG maps to ENAMETOOLONG */
+      /* WSAEHOSTDOWN is EHOSTDOWN */
+      /* WSAEHOSTUNREACH is EHOSTUNREACH */
+      /* WSAENOTEMPTY maps to ENOTEMPTY */
+      /* WSAEPROCLIM is EPROCLIM */
+      /* WSAEUSERS is EUSERS */
+      /* WSAEDQUOT is EDQUOT */
+      /* WSAESTALE is ESTALE */
+      /* WSAEREMOTE is EREMOTE */
+    case WSASYSNOTREADY:
+      msg = "Network subsystem is unavailable";
+      break;
+    case WSAVERNOTSUPPORTED:
+      msg = "Winsock.dll version out of range";
+      break;
+    case WSANOTINITIALISED:
+      msg = "Successful WSAStartup not yet performed";
+      break;
+    case WSAEDISCON:
+      msg = "Graceful shutdown in progress";
+      break;
+    case WSAENOMORE: case WSA_E_NO_MORE:
+      msg = "No more results";
+      break;
+    case WSAECANCELLED: case WSA_E_CANCELLED:
+      msg = "Call was canceled";
+      break;
+    case WSAEINVALIDPROCTABLE:
+      msg = "Procedure call table is invalid";
+      break;
+    case WSAEINVALIDPROVIDER:
+      msg = "Service provider is invalid";
+      break;
+    case WSAEPROVIDERFAILEDINIT:
+      msg = "Service provider failed to initialize";
+      break;
+    case WSASYSCALLFAILURE:
+      msg = "System call failure";
+      break;
+    case WSASERVICE_NOT_FOUND:
+      msg = "Service not found";
+      break;
+    case WSATYPE_NOT_FOUND:
+      msg = "Class type not found";
+      break;
+    case WSAEREFUSED:
+      msg = "Database query was refused";
+      break;
+    case WSAHOST_NOT_FOUND:
+      msg = "Host not found";
+      break;
+    case WSATRY_AGAIN:
+      msg = "Nonauthoritative host not found";
+      break;
+    case WSANO_RECOVERY:
+      msg = "Nonrecoverable error";
+      break;
+    case WSANO_DATA:
+      msg = "Valid name, no data record of requested type";
+      break;
+      /* WSA_QOS_* omitted */
+#  endif
+# endif
+
+# if GNULIB_defined_ENOMSG
+    case ENOMSG:
+      msg = "No message of desired type";
+      break;
+# endif
+
+# if GNULIB_defined_EIDRM
+    case EIDRM:
+      msg = "Identifier removed";
+      break;
+# endif
+
+# if GNULIB_defined_ENOLINK
+    case ENOLINK:
+      msg = "Link has been severed";
+      break;
+# endif
+
+# if GNULIB_defined_EPROTO
+    case EPROTO:
+      msg = "Protocol error";
+      break;
+# endif
+
+# if GNULIB_defined_EMULTIHOP
+    case EMULTIHOP:
+      msg = "Multihop attempted";
+      break;
+# endif
+
+# if GNULIB_defined_EBADMSG
+    case EBADMSG:
+      msg = "Bad message";
+      break;
+# endif
+
+# if GNULIB_defined_EOVERFLOW
+    case EOVERFLOW:
+      msg = "Value too large for defined data type";
+      break;
+# endif
+
+# if GNULIB_defined_ENOTSUP
+    case ENOTSUP:
+      msg = "Not supported";
+      break;
+# endif
+
+# if GNULIB_defined_ESTALE
+    case ESTALE:
+      msg = "Stale NFS file handle";
+      break;
+# endif
+
+# if GNULIB_defined_EDQUOT
+    case EDQUOT:
+      msg = "Disk quota exceeded";
+      break;
+# endif
+
+# if GNULIB_defined_ECANCELED
+    case ECANCELED:
+      msg = "Operation canceled";
+      break;
+# endif
+    }
+#endif
+
+  return msg;
+}
diff --git a/lib/strerror-override.h b/lib/strerror-override.h
new file mode 100644
index 0000000..0718b53
--- /dev/null
+++ b/lib/strerror-override.h
@@ -0,0 +1,48 @@
+/* strerror-override.h --- POSIX compatible system error routine
+
+   Copyright (C) 2010-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 <http://www.gnu.org/licenses/>.  */
+
+#ifndef _GL_STRERROR_OVERRIDE_H
+# define _GL_STRERROR_OVERRIDE_H
+
+# include <string.h>
+
+/* Reasonable buffer size that should never trigger ERANGE; if this
+   proves too small, we intentionally abort(), to remind us to fix
+   this value.  */
+# define STACKBUF_LEN 256
+
+/* If ERRNUM maps to an errno value defined by gnulib, return a string
+   describing the error.  Otherwise return NULL.  */
+# if GNULIB_defined_ETXTBSY \
+    || GNULIB_defined_ESOCK \
+    || GNULIB_defined_ENOMSG \
+    || GNULIB_defined_EIDRM \
+    || GNULIB_defined_ENOLINK \
+    || GNULIB_defined_EPROTO \
+    || GNULIB_defined_EMULTIHOP \
+    || GNULIB_defined_EBADMSG \
+    || GNULIB_defined_EOVERFLOW \
+    || GNULIB_defined_ENOTSUP \
+    || GNULIB_defined_ESTALE \
+    || GNULIB_defined_EDQUOT \
+    || GNULIB_defined_ECANCELED
+extern const char *strerror_override (int errnum);
+# else
+#  define strerror_override(ignored) NULL
+# endif
+
+#endif /* _GL_STRERROR_OVERRIDE_H */
diff --git a/lib/strerror.c b/lib/strerror.c
index 9d1f165..8c41179 100644
--- a/lib/strerror.c
+++ b/lib/strerror.c
@@ -23,11 +23,45 @@
 #include <errno.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>

 #include "intprops.h"
+#include "strerror-override.h"
 #include "verify.h"

 /* Use the system functions, not the gnulib overrides in this file.  */
 #undef sprintf

-#include "strerror-impl.h"
+char *
+strerror (int n)
+#undef strerror
+{
+  static char buf[STACKBUF_LEN];
+  size_t len;
+
+  /* Cast away const, due to the historical signature of strerror;
+     callers should not be modifying the string.  */
+  const char *msg = strerror_override (n);
+  if (msg)
+    return (char *) msg;
+
+  /* Our strerror_r implementation might use the system's strerror
+     buffer, so all other clients of strerror have to see the error
+     copied into a buffer that we manage.  */
+  msg = strerror (n);
+  if (!msg || !*msg)
+    {
+      static char const fmt[] = "Unknown error %d";
+      verify (sizeof buf >= sizeof (fmt) + INT_STRLEN_BOUND (n));
+      sprintf (buf, fmt, n);
+      errno = EINVAL;
+      return buf;
+    }
+
+  /* Fix STACKBUF_LEN if this ever aborts.  */
+  len = strlen (msg);
+  if (sizeof buf <= len)
+    abort ();
+
+  return memcpy (buf, msg, len + 1);
+}
diff --git a/lib/strerror_r.c b/lib/strerror_r.c
index f9242e3..93e33fa 100644
--- a/lib/strerror_r.c
+++ b/lib/strerror_r.c
@@ -29,16 +29,7 @@
 #include <stdio.h>
 #include <stdlib.h>

-#if GNULIB_defined_ESOCK /* native Windows platforms */
-# if HAVE_WINSOCK2_H
-#  include <winsock2.h>
-# 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
+#include "strerror-override.h"

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

@@ -134,316 +125,13 @@ strerror_r (int errnum, char *buf, size_t buflen)
     }
   *buf = '\0';

-#if GNULIB_defined_ETXTBSY \
-    || GNULIB_defined_ESOCK \
-    || GNULIB_defined_ENOMSG \
-    || GNULIB_defined_EIDRM \
-    || GNULIB_defined_ENOLINK \
-    || GNULIB_defined_EPROTO \
-    || GNULIB_defined_EMULTIHOP \
-    || GNULIB_defined_EBADMSG \
-    || GNULIB_defined_EOVERFLOW \
-    || GNULIB_defined_ENOTSUP \
-    || GNULIB_defined_ESTALE \
-    || GNULIB_defined_EDQUOT \
-    || GNULIB_defined_ECANCELED
+  /* Check for gnulib overrides.  */
   {
-    char const *msg = NULL;
-    /* These error messages are taken from glibc/sysdeps/gnu/errlist.c.  */
-    switch (errnum)
-      {
-# if GNULIB_defined_ETXTBSY
-      case ETXTBSY:
-        msg = "Text file busy";
-        break;
-# endif
-
-# if GNULIB_defined_ESOCK /* native Windows platforms */
-      /* EWOULDBLOCK is the same as EAGAIN.  */
-      case EINPROGRESS:
-        msg = "Operation now in progress";
-        break;
-      case EALREADY:
-        msg = "Operation already in progress";
-        break;
-      case ENOTSOCK:
-        msg = "Socket operation on non-socket";
-        break;
-      case EDESTADDRREQ:
-        msg = "Destination address required";
-        break;
-      case EMSGSIZE:
-        msg = "Message too long";
-        break;
-      case EPROTOTYPE:
-        msg = "Protocol wrong type for socket";
-        break;
-      case ENOPROTOOPT:
-        msg = "Protocol not available";
-        break;
-      case EPROTONOSUPPORT:
-        msg = "Protocol not supported";
-        break;
-      case ESOCKTNOSUPPORT:
-        msg = "Socket type not supported";
-        break;
-      case EOPNOTSUPP:
-        msg = "Operation not supported";
-        break;
-      case EPFNOSUPPORT:
-        msg = "Protocol family not supported";
-        break;
-      case EAFNOSUPPORT:
-        msg = "Address family not supported by protocol";
-        break;
-      case EADDRINUSE:
-        msg = "Address already in use";
-        break;
-      case EADDRNOTAVAIL:
-        msg = "Cannot assign requested address";
-        break;
-      case ENETDOWN:
-        msg = "Network is down";
-        break;
-      case ENETUNREACH:
-        msg = "Network is unreachable";
-        break;
-      case ENETRESET:
-        msg = "Network dropped connection on reset";
-        break;
-      case ECONNABORTED:
-        msg = "Software caused connection abort";
-        break;
-      case ECONNRESET:
-        msg = "Connection reset by peer";
-        break;
-      case ENOBUFS:
-        msg = "No buffer space available";
-        break;
-      case EISCONN:
-        msg = "Transport endpoint is already connected";
-        break;
-      case ENOTCONN:
-        msg = "Transport endpoint is not connected";
-        break;
-      case ESHUTDOWN:
-        msg = "Cannot send after transport endpoint shutdown";
-        break;
-      case ETOOMANYREFS:
-        msg = "Too many references: cannot splice";
-        break;
-      case ETIMEDOUT:
-        msg = "Connection timed out";
-        break;
-      case ECONNREFUSED:
-        msg = "Connection refused";
-        break;
-      case ELOOP:
-        msg = "Too many levels of symbolic links";
-        break;
-      case EHOSTDOWN:
-        msg = "Host is down";
-        break;
-      case EHOSTUNREACH:
-        msg = "No route to host";
-        break;
-      case EPROCLIM:
-        msg = "Too many processes";
-        break;
-      case EUSERS:
-        msg = "Too many users";
-        break;
-      case EDQUOT:
-        msg = "Disk quota exceeded";
-        break;
-      case ESTALE:
-        msg = "Stale NFS file handle";
-        break;
-      case EREMOTE:
-        msg = "Object is remote";
-        break;
-#  if HAVE_WINSOCK2_H
-      /* WSA_INVALID_HANDLE maps to EBADF */
-      /* WSA_NOT_ENOUGH_MEMORY maps to ENOMEM */
-      /* WSA_INVALID_PARAMETER maps to EINVAL */
-      case WSA_OPERATION_ABORTED:
-        msg = "Overlapped operation aborted";
-        break;
-      case WSA_IO_INCOMPLETE:
-        msg = "Overlapped I/O event object not in signaled state";
-        break;
-      case WSA_IO_PENDING:
-        msg = "Overlapped operations will complete later";
-        break;
-      /* WSAEINTR maps to EINTR */
-      /* WSAEBADF maps to EBADF */
-      /* WSAEACCES maps to EACCES */
-      /* WSAEFAULT maps to EFAULT */
-      /* WSAEINVAL maps to EINVAL */
-      /* WSAEMFILE maps to EMFILE */
-      /* WSAEWOULDBLOCK maps to EWOULDBLOCK */
-      /* WSAEINPROGRESS is EINPROGRESS */
-      /* WSAEALREADY is EALREADY */
-      /* WSAENOTSOCK is ENOTSOCK */
-      /* WSAEDESTADDRREQ is EDESTADDRREQ */
-      /* WSAEMSGSIZE is EMSGSIZE */
-      /* WSAEPROTOTYPE is EPROTOTYPE */
-      /* WSAENOPROTOOPT is ENOPROTOOPT */
-      /* WSAEPROTONOSUPPORT is EPROTONOSUPPORT */
-      /* WSAESOCKTNOSUPPORT is ESOCKTNOSUPPORT */
-      /* WSAEOPNOTSUPP is EOPNOTSUPP */
-      /* WSAEPFNOSUPPORT is EPFNOSUPPORT */
-      /* WSAEAFNOSUPPORT is EAFNOSUPPORT */
-      /* WSAEADDRINUSE is EADDRINUSE */
-      /* WSAEADDRNOTAVAIL is EADDRNOTAVAIL */
-      /* WSAENETDOWN is ENETDOWN */
-      /* WSAENETUNREACH is ENETUNREACH */
-      /* WSAENETRESET is ENETRESET */
-      /* WSAECONNABORTED is ECONNABORTED */
-      /* WSAECONNRESET is ECONNRESET */
-      /* WSAENOBUFS is ENOBUFS */
-      /* WSAEISCONN is EISCONN */
-      /* WSAENOTCONN is ENOTCONN */
-      /* WSAESHUTDOWN is ESHUTDOWN */
-      /* WSAETOOMANYREFS is ETOOMANYREFS */
-      /* WSAETIMEDOUT is ETIMEDOUT */
-      /* WSAECONNREFUSED is ECONNREFUSED */
-      /* WSAELOOP is ELOOP */
-      /* WSAENAMETOOLONG maps to ENAMETOOLONG */
-      /* WSAEHOSTDOWN is EHOSTDOWN */
-      /* WSAEHOSTUNREACH is EHOSTUNREACH */
-      /* WSAENOTEMPTY maps to ENOTEMPTY */
-      /* WSAEPROCLIM is EPROCLIM */
-      /* WSAEUSERS is EUSERS */
-      /* WSAEDQUOT is EDQUOT */
-      /* WSAESTALE is ESTALE */
-      /* WSAEREMOTE is EREMOTE */
-      case WSASYSNOTREADY:
-        msg = "Network subsystem is unavailable";
-        break;
-      case WSAVERNOTSUPPORTED:
-        msg = "Winsock.dll version out of range";
-        break;
-      case WSANOTINITIALISED:
-        msg = "Successful WSAStartup not yet performed";
-        break;
-      case WSAEDISCON:
-        msg = "Graceful shutdown in progress";
-        break;
-      case WSAENOMORE: case WSA_E_NO_MORE:
-        msg = "No more results";
-        break;
-      case WSAECANCELLED: case WSA_E_CANCELLED:
-        msg = "Call was canceled";
-        break;
-      case WSAEINVALIDPROCTABLE:
-        msg = "Procedure call table is invalid";
-        break;
-      case WSAEINVALIDPROVIDER:
-        msg = "Service provider is invalid";
-        break;
-      case WSAEPROVIDERFAILEDINIT:
-        msg = "Service provider failed to initialize";
-        break;
-      case WSASYSCALLFAILURE:
-        msg = "System call failure";
-        break;
-      case WSASERVICE_NOT_FOUND:
-        msg = "Service not found";
-        break;
-      case WSATYPE_NOT_FOUND:
-        msg = "Class type not found";
-        break;
-      case WSAEREFUSED:
-        msg = "Database query was refused";
-        break;
-      case WSAHOST_NOT_FOUND:
-        msg = "Host not found";
-        break;
-      case WSATRY_AGAIN:
-        msg = "Nonauthoritative host not found";
-        break;
-      case WSANO_RECOVERY:
-        msg = "Nonrecoverable error";
-        break;
-      case WSANO_DATA:
-        msg = "Valid name, no data record of requested type";
-        break;
-      /* WSA_QOS_* omitted */
-#  endif
-# endif
-
-# if GNULIB_defined_ENOMSG
-      case ENOMSG:
-        msg = "No message of desired type";
-        break;
-# endif
-
-# if GNULIB_defined_EIDRM
-      case EIDRM:
-        msg = "Identifier removed";
-        break;
-# endif
-
-# if GNULIB_defined_ENOLINK
-      case ENOLINK:
-        msg = "Link has been severed";
-        break;
-# endif
-
-# if GNULIB_defined_EPROTO
-      case EPROTO:
-        msg = "Protocol error";
-        break;
-# endif
-
-# if GNULIB_defined_EMULTIHOP
-      case EMULTIHOP:
-        msg = "Multihop attempted";
-        break;
-# endif
-
-# if GNULIB_defined_EBADMSG
-      case EBADMSG:
-        msg = "Bad message";
-        break;
-# endif
-
-# if GNULIB_defined_EOVERFLOW
-      case EOVERFLOW:
-        msg = "Value too large for defined data type";
-        break;
-# endif
-
-# if GNULIB_defined_ENOTSUP
-      case ENOTSUP:
-        msg = "Not supported";
-        break;
-# endif
-
-# if GNULIB_defined_ESTALE
-      case ESTALE:
-        msg = "Stale NFS file handle";
-        break;
-# endif
-
-# if GNULIB_defined_EDQUOT
-      case EDQUOT:
-        msg = "Disk quota exceeded";
-        break;
-# endif
-
-# if GNULIB_defined_ECANCELED
-      case ECANCELED:
-        msg = "Operation canceled";
-        break;
-# endif
-      }
+    char const *msg = strerror_override (errnum);

     if (msg)
       return safe_copy (buf, buflen, msg);
   }
-#endif

   {
     int ret;
diff --git a/m4/errno_h.m4 b/m4/errno_h.m4
index a6d37f3..3390bf9 100644
--- a/m4/errno_h.m4
+++ b/m4/errno_h.m4
@@ -1,4 +1,4 @@
-# errno_h.m4 serial 9
+# errno_h.m4 serial 10
 dnl Copyright (C) 2004, 2006, 2008-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,
@@ -51,6 +51,7 @@ booboo
     ERRNO_H=''
   else
     gl_NEXT_HEADERS([errno.h])
+    AC_LIBOBJ([strerror-override])
     ERRNO_H='errno.h'
   fi
   AC_SUBST([ERRNO_H])
diff --git a/modules/errno b/modules/errno
index 2fee35d..e654088 100644
--- a/modules/errno
+++ b/modules/errno
@@ -3,6 +3,8 @@ A POSIX-like <errno.h>.

 Files:
 lib/errno.in.h
+lib/strerror-override.h
+lib/strerror-override.c
 m4/errno_h.m4

 Depends-on:
diff --git a/modules/strerror b/modules/strerror
index 0e7a254..d4d9c79 100644
--- a/modules/strerror
+++ b/modules/strerror
@@ -3,7 +3,6 @@ strerror() function: return string describing error code.

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

 Depends-on:
@@ -11,7 +10,6 @@ string
 errno            [test $REPLACE_STRERROR = 1]
 intprops         [test $REPLACE_STRERROR = 1]
 verify           [test $REPLACE_STRERROR = 1]
-strerror_r-posix [test $REPLACE_STRERROR = 1]

 configure.ac:
 gl_FUNC_STRERROR
-- 
1.7.4.4

Re: [PATCHv2] strerror: drop strerror_r dependency

[Bruno Haible]

Hi Eric,

> Since the errno module is responsible for introducing replacement
> errno values, it should also be responsible for translating those
> new values into strings.  And by moving the replacements into a
> file managed by the errno, we can then break the dependency between
> strerror and strerror_r, so that strerror no longer drags in
> multi-threading modules required by strerror_r.

Creating a separate strerror-override.[hc] is reasonable.

However, I'm not in favour of attaching object files to modules that
are meant to provide a header file. For two reasons:
  - Not everyone who needs 'errno' also needs strerror() or strerror_r().
    There are many uses, from iconv() to <pthread.h>, of errno values
    without the need of a conversion to string.
  - This change causes trouble for the 'relocatable-prog-wrapper'
    module. This module is meant to have all its source code compiled
    on the fly and not provide any object file into libgnu.a (i.e.
    no AC_LIBOBJ invocations).

Please, can we revisit this?

Another point with this patch is that it leads to a link error of the
gettext build on OpenBSD 4.4:

/bin/sh ../libtool  --tag=CC    --mode=link gcc -std=gnu99  -g -O2    -o gettextpo-1-prg gettextpo_1_prg-gettextpo-1-prg.o ../libgettextpo/libgettextpo.la ../intl/libintl.la 
libtool: link: gcc -std=gnu99 -g -O2 -o .libs/gettextpo-1-prg gettextpo_1_prg-gettextpo-1-prg.o  -L/home/haible/multibuild-1028/openbsd44/gettext-0.18.1/gettext-tools/intl/.libs -L../libgettextpo/.libs -lgettextpo -L/usr/local/lib -L../intl/.libs -lintl -liconv -Wl,-rpath,/usr/local/lib
../libgettextpo/.libs/libgettextpo.so.5.1: warning: strcpy() is almost always misused, please use strlcpy()
../libgettextpo/.libs/libgettextpo.so.5.1: warning: sprintf() is often misused, please use snprintf()
../libgettextpo/.libs/libgettextpo.so.5.1: undefined reference to `libgettextpo_strerror_override'
collect2: ld returned 1 exit status
*** Error code 1

This undefined reference occurs because the config.h file for libgettextpo
contains definitions like
  #define strerror_override libgettextpo_strerror_override
that achieve namespace cleanliness of the shared libraries, and
lib/strerror-override.c breaks this
  1. by omitting the mandatory '#include <confog.h>,
  2. by explicitly doing a #undef.

Both needs to be fixed. I plan to push this once savannah is back:


2011-06-03  Bruno Haible  <bruno@clisp.org>

	strerror-override: Don't disable symbol renamings.
	* lib/strerror-override.c: Include config.h.
	(strerror_override): Don't undefine.

--- lib/strerror-override.c.orig	Sat Jun  4 02:17:13 2011
+++ lib/strerror-override.c	Sat Jun  4 02:16:46 2011
@@ -17,6 +17,8 @@
 
 /* Written by Bruno Haible <bruno@clisp.org>, 2010.  */
 
+#include <config.h>
+
 #include "strerror-override.h"
 
 #include <errno.h>
@@ -27,10 +29,6 @@
 # endif
 #endif
 
-/* This undefine allows testing with gl_cv_header_errno_h_complete=no on
-   a system that otherwise has a complete errno.h.  */
-#undef strerror_override
-
 /* If ERRNUM maps to an errno value defined by gnulib, return a string
    describing the error.  Otherwise return NULL.  */
 const char *

-- 
In memoriam Leo Katzenberger <http://en.wikipedia.org/wiki/Katzenberger_Trial>

Re: [PATCHv2] strerror: drop strerror_r dependency

[Bruno Haible]

> 2011-06-03  Bruno Haible  <bruno@clisp.org>
> 
> 	strerror-override: Don't disable symbol renamings.
> 	* lib/strerror-override.c: Include config.h.
> 	(strerror_override): Don't undefine.

This patch alone leads to a compilation error, because at the point of the
definition of the function in strerror-override.c, strerror_override expands
to NULL! Obviously, one needs to include <errno.h> before testing
GNULIB_defined_ETXTBSY. And including <string.h> is overkill just for getting
NULL defined - <stddef.h> is the minimal header.


2011-06-04  Bruno Haible  <bruno@clisp.org>

	strerror-override: Don't disable symbol renamings.
	* lib/strerror-override.h: Include errno.h and stddef.h, not string.h.
	* lib/strerror-override.c: Include config.h.
	(strerror_override): Don't undefine.

--- lib/strerror-override.c.orig	Sat Jun  4 10:54:19 2011
+++ lib/strerror-override.c	Sat Jun  4 02:16:46 2011
@@ -17,6 +17,8 @@
 
 /* Written by Bruno Haible <bruno@clisp.org>, 2010.  */
 
+#include <config.h>
+
 #include "strerror-override.h"
 
 #include <errno.h>
@@ -27,10 +29,6 @@
 # endif
 #endif
 
-/* This undefine allows testing with gl_cv_header_errno_h_complete=no on
-   a system that otherwise has a complete errno.h.  */
-#undef strerror_override
-
 /* If ERRNUM maps to an errno value defined by gnulib, return a string
    describing the error.  Otherwise return NULL.  */
 const char *
--- lib/strerror-override.h.orig	Sat Jun  4 10:54:19 2011
+++ lib/strerror-override.h	Sat Jun  4 04:32:21 2011
@@ -18,7 +18,8 @@
 #ifndef _GL_STRERROR_OVERRIDE_H
 # define _GL_STRERROR_OVERRIDE_H
 
-# include <string.h>
+# include <errno.h>
+# include <stddef.h>
 
 /* Reasonable buffer size that should never trigger ERANGE; if this
    proves too small, we intentionally abort(), to remind us to fix

-- 
In memoriam Mordechai Gebirtig <http://en.wikipedia.org/wiki/Mordechai_Gebirtig>

Re: [PATCH 1/2] perror: call strerror_r directly

[Bruno Haible]

Hi Eric,

> > 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.
> 
> No comments, so I've pushed this.

If I didn't have time to review this patch, someone else ought to review it
instead of me.

This patch breaks compilation of module 'perror':

  perror.c:38:27: strerror-impl.h: No such file or directory
  *** Error code 1

I'm fixing it like this:


2011-06-04  Bruno Haible  <bruno@clisp.org>

	perror: Fix compilation error.
	* lib/perror.c: Don't include intprops.h, verify.h, strerror-impl.h.
	Undefine fprintf, not sprintf.
	* modules/perror (Depends-on): Remove intprops, verify.

--- lib/perror.c.orig	Sat Jun  4 12:51:16 2011
+++ lib/perror.c	Sat Jun  4 12:47:18 2011
@@ -24,18 +24,8 @@
 #include <stdlib.h>
 #include <string.h>
 
-#include "intprops.h"
-#include "verify.h"
-
 /* Use the system functions, not the gnulib overrides in this file.  */
-#undef sprintf
-
-/* my_strerror (errnum) is equivalent to strerror (errnum).
-   But it uses its own buffer, not the one from strerror().  */
-#define STATIC static
-#undef strerror
-#define strerror my_strerror
-#include "strerror-impl.h"
+#undef fprintf
 
 void
 perror (const char *string)
--- modules/perror.orig	Sat Jun  4 12:51:16 2011
+++ modules/perror	Sat Jun  4 12:51:09 2011
@@ -8,8 +8,6 @@
 Depends-on:
 stdio
 errno            [test $REPLACE_PERROR = 1]
-intprops         [test $REPLACE_PERROR = 1]
-verify           [test $REPLACE_PERROR = 1]
 strerror_r-posix [test $REPLACE_PERROR = 1]
 
 configure.ac:

-- 
In memoriam Mordechai Gebirtig <http://en.wikipedia.org/wiki/Mordechai_Gebirtig>

Re: [PATCHv2] strerror: drop strerror_r dependency

[Bruno Haible]

Eric Blake wrote:
> * lib/strerror-impl.h: Delete.

The comments still refer to this file. Fixing it like this:


2011-06-04  Bruno Haible  <bruno@clisp.org>

	strerror_r: Fix comments.
	* lib/strerror_r.c (strerror_r): Update comments after 2011-06-01
	commit.

--- lib/strerror_r.c.orig	Sat Jun  4 12:59:43 2011
+++ lib/strerror_r.c	Sat Jun  4 12:56:09 2011
@@ -181,9 +181,8 @@
       {
         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)
+          /* STACKBUF_LEN should have been large enough.  */
           abort ();
         safe_copy (buf, buflen, stackbuf);
       }
@@ -198,7 +197,7 @@
         size_t len;
         strerror_r (errnum, stackbuf, sizeof stackbuf);
         len = strlen (stackbuf);
-        /* stackbuf should have been large enough.  */
+        /* STACKBUF_LEN should have been large enough.  */
         if (len + 1 == sizeof stackbuf)
           abort ();
         if (buflen <= len)
-- 
In memoriam Mordechai Gebirtig <http://en.wikipedia.org/wiki/Mordechai_Gebirtig>

Re: [PATCHv2] strerror: drop strerror_r dependency

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1516 bytes --]

On 06/03/2011 06:28 PM, Bruno Haible wrote:
> Hi Eric,
> 
>> Since the errno module is responsible for introducing replacement
>> errno values, it should also be responsible for translating those
>> new values into strings.  And by moving the replacements into a
>> file managed by the errno, we can then break the dependency between
>> strerror and strerror_r, so that strerror no longer drags in
>> multi-threading modules required by strerror_r.
> 
> Creating a separate strerror-override.[hc] is reasonable.
> 
> However, I'm not in favour of attaching object files to modules that
> are meant to provide a header file. For two reasons:
>   - Not everyone who needs 'errno' also needs strerror() or strerror_r().
>     There are many uses, from iconv() to <pthread.h>, of errno values
>     without the need of a conversion to string.
>   - This change causes trouble for the 'relocatable-prog-wrapper'
>     module. This module is meant to have all its source code compiled
>     on the fly and not provide any object file into libgnu.a (i.e.
>     no AC_LIBOBJ invocations).
> 
> Please, can we revisit this?

You bet.  Looks like we need to go back to my first idea of creating a
separate strerror-override module (needed because strerror and
strerror_r-posix are now independent, but both need the overrides, so
the overrides have to be AC_LIBOBJ at the first client).

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [PATCHv2] strerror: drop strerror_r dependency

[Bruno Haible]

Eric Blake wrote:
> both need the overrides, so
> the overrides have to be AC_LIBOBJ at the first client).

You can have more than one AC_LIBOBJ invocation of the same file. Duplicates
removed automatically, cf. gnulib-tool line 3758.

Bruno
-- 
In memoriam Robert F. Kennedy <http://en.wikipedia.org/wiki/Robert_F._Kennedy>

Re: [PATCHv2] strerror: drop strerror_r dependency

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1014 bytes --]

On 06/06/2011 02:37 PM, Bruno Haible wrote:
> Eric Blake wrote:
>> both need the overrides, so
>> the overrides have to be AC_LIBOBJ at the first client).
> 
> You can have more than one AC_LIBOBJ invocation of the same file. Duplicates
> removed automatically, cf. gnulib-tool line 3758.

But only if they are both in libs or both in tests.  If you have
strerror in libs and strerror_r in tests, and both try to do AC_LIBOBJ,
then you get into the funky situation where strerror-overrided.c is not
in the tests directory and the build fails.  Compare to the problem we
were solving earlier with fclose vs. fflush being in separate
directories, and the AC_LIBOBJ([fflush]) giving us grief.

Which is why we need a separate module as prereq for both strerror and
strerror_r-posix, so that the AC_LIBOBJ only occurs in one directory
corresponding to where strerror-override.c is placed.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [PATCHv2] strerror: drop strerror_r dependency

[Bruno Haible]

Eric Blake wrote:
> ...  Compare to the problem we
> were solving earlier with fclose vs. fflush being in separate
> directories, and the AC_LIBOBJ([fflush]) giving us grief.
> 
> Which is why we need a separate module as prereq for both strerror and
> strerror_r-posix, so that the AC_LIBOBJ only occurs in one directory
> corresponding to where strerror-override.c is placed.

A separate module is generally a good thing in a situation like this, yes.
I didn't mean to dissuade you from creating the separate module.

> If you have
> strerror in libs and strerror_r in tests, and both try to do AC_LIBOBJ,
> then you get into the funky situation where strerror-overrided.c is not
> in the tests directory and the build fails.

It will work *if* both strerror and strerror_r-posix have
lib/strerror-overrided.c among their file list. The rule about AC_LIBOBJ is:
The module which triggers an AC_LIBOBJ([foo]) must have lib/foo.c in its
file list. Look in gnulib-tool, function func_modules_to_filelist_separately,
how the final file list will be computed.

It's done correctly in module 'malloc-posix' and 'malloc-gnu', for example.
But there are many places in gnulib which we need to fix.

Bruno
-- 
In memoriam Robert F. Kennedy <http://en.wikipedia.org/wiki/Robert_F._Kennedy>

[PATCH 1/2] strerror-override: avoid bloating errno module

[Eric Blake]

* modules/errno (Files, configure.ac): Move replacement strings...
* modules/strerror-override: ...to new module.
* modules/strerror (Depends-on): Add strerror-override.
* modules/strerror_r-posix (Depends-on): Likewise.
* MODULES.html.sh: Document new module.
Reported by Bruno Haible.

Signed-off-by: Eric Blake <eblake@redhat.com>
---

I went with the separate module approach.

 ChangeLog                 |   10 ++++++++++
 MODULES.html.sh           |    1 +
 modules/errno             |    5 -----
 modules/strerror          |    1 +
 modules/strerror-override |   26 ++++++++++++++++++++++++++
 modules/strerror_r-posix  |    1 +
 6 files changed, 39 insertions(+), 5 deletions(-)
 create mode 100644 modules/strerror-override

diff --git a/ChangeLog b/ChangeLog
index 799cdb9..363ee1f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2011-06-06  Eric Blake  <eblake@redhat.com>
+
+	strerror-override: avoid bloating errno module
+	* modules/errno (Files, configure.ac): Move replacement strings...
+	* modules/strerror-override: ...to new module.
+	* modules/strerror (Depends-on): Add strerror-override.
+	* modules/strerror_r-posix (Depends-on): Likewise.
+	* MODULES.html.sh: Document new module.
+	Reported by Bruno Haible.
+
 2011-06-06  Bruno Haible  <bruno@clisp.org>

 	spawn-pipe tests: Rename program.
diff --git a/MODULES.html.sh b/MODULES.html.sh
index f929ecf..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

diff --git a/modules/errno b/modules/errno
index 2640ec8..2fee35d 100644
--- a/modules/errno
+++ b/modules/errno
@@ -3,8 +3,6 @@ A POSIX-like <errno.h>.

 Files:
 lib/errno.in.h
-lib/strerror-override.h
-lib/strerror-override.c
 m4/errno_h.m4

 Depends-on:
@@ -12,9 +10,6 @@ include_next

 configure.ac:
 gl_HEADER_ERRNO_H
-if test -n "$ERRNO_H"; then
-  AC_LIBOBJ([strerror-override])
-fi

 Makefile.am:
 BUILT_SOURCES += $(ERRNO_H)
diff --git a/modules/strerror b/modules/strerror
index d4d9c79..3c83cf9 100644
--- a/modules/strerror
+++ b/modules/strerror
@@ -9,6 +9,7 @@ Depends-on:
 string
 errno            [test $REPLACE_STRERROR = 1]
 intprops         [test $REPLACE_STRERROR = 1]
+strerror-override [test $REPLACE_STRERROR = 1]
 verify           [test $REPLACE_STRERROR = 1]

 configure.ac:
diff --git a/modules/strerror-override b/modules/strerror-override
new file mode 100644
index 0000000..a31e8a1
--- /dev/null
+++ b/modules/strerror-override
@@ -0,0 +1,26 @@
+Description:
+strerror_override() function: provide strings for gnulib-specific errno values
+
+Files:
+lib/strerror-override.h
+lib/strerror-override.c
+
+Depends-on:
+errno
+
+configure.ac:
+AC_REQUIRE([gl_HEADER_ERRNO_H])
+if test -n "$ERRNO_H"; then
+  AC_LIBOBJ([strerror-override])
+fi
+
+Makefile.am:
+
+Include:
+"strerror-override.h"
+
+License:
+LGPLv2+
+
+Maintainer:
+all
diff --git a/modules/strerror_r-posix b/modules/strerror_r-posix
index 90c9806..c7f18b2 100644
--- a/modules/strerror_r-posix
+++ b/modules/strerror_r-posix
@@ -10,6 +10,7 @@ string
 extensions
 errno           [test $HAVE_DECL_STRERROR_R = 0 || test $REPLACE_STRERROR_R = 1]
 lock            [test $HAVE_DECL_STRERROR_R = 0 || test $REPLACE_STRERROR_R = 1]
+strerror-override [test $HAVE_DECL_STRERROR_R = 0 || test $REPLACE_STRERROR_R = 1]

 configure.ac:
 gl_FUNC_STRERROR_R
-- 
1.7.4.4

[PATCH 2/2] strerror: work around FreeBSD bug

[Eric Blake]

Breaking strerror away from strerror_r re-exposed the FreeBSD
strerror(0) bug.

* lib/strerror.c (strerror): Special case 0.
Reported by Bruno Haible.

Signed-off-by: Eric Blake <eblake@redhat.com>
---

This fixes test-strerror on FreeBSD, again.

I still need to work on perror, but I'm pushing this in the meantime.

 ChangeLog      |    4 ++++
 lib/strerror.c |   14 +++++++++++++-
 2 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 363ee1f..35497d0 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
 2011-06-06  Eric Blake  <eblake@redhat.com>

+	strerror: work around FreeBSD bug
+	* lib/strerror.c (strerror): Special case 0.
+	Reported by Bruno Haible.
+
 	strerror-override: avoid bloating errno module
 	* modules/errno (Files, configure.ac): Move replacement strings...
 	* modules/strerror-override: ...to new module.
diff --git a/lib/strerror.c b/lib/strerror.c
index 8c41179..4dc0b65 100644
--- a/lib/strerror.c
+++ b/lib/strerror.c
@@ -45,10 +45,22 @@ strerror (int n)
   if (msg)
     return (char *) msg;

+  /* FreeBSD rejects 0; see http://austingroupbugs.net/view.php?id=382.  */
+  if (n)
+    msg = strerror (n);
+  else
+    {
+      int saved_errno = errno;
+      errno = 0;
+      msg = strerror (n);
+      if (errno)
+        msg = "Success";
+      errno = saved_errno;
+    }
+
   /* Our strerror_r implementation might use the system's strerror
      buffer, so all other clients of strerror have to see the error
      copied into a buffer that we manage.  */
-  msg = strerror (n);
   if (!msg || !*msg)
     {
       static char const fmt[] = "Unknown error %d";
-- 
1.7.4.4

Re: [PATCH 2/2] strerror: work around FreeBSD bug

[Bruno Haible]

Eric Blake wrote:
> * lib/strerror.c (strerror): Special case 0.

Thanks.

On MacOS X 10.5, I'm still seeing these failures:

  test-strerror.c:63: assertion failed
  FAIL: test-strerror

  test-perror2.c:112: assertion failed
  FAIL: test-perror2

Bruno
-- 
In memoriam The Jews from the Braslav ghetto <http://www.geschichteinchronologie.ch/eu/BSSR/EncJud_juden-in-kl-staedten-ENGL.html>

Re: [PATCH 2/2] strerror: work around FreeBSD bug

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 776 bytes --]

On 06/07/2011 03:29 AM, Bruno Haible wrote:
> Eric Blake wrote:
>> * lib/strerror.c (strerror): Special case 0.
> 
> Thanks.
> 
> On MacOS X 10.5, I'm still seeing these failures:
> 
>   test-strerror.c:63: assertion failed
>   FAIL: test-strerror

strerror(0) misbehavior.  Hmm, while FreeBSD was setting EINVAL to make
the failure explicit, MacOS is claiming success while still outputting
"Unknown error: 0".  I'll fix that with a strstr() test in the .m4 files.

> 
>   test-perror2.c:112: assertion failed
>   FAIL: test-perror2

Buffer clobbering; given that I just fixed that in cygwin, it shouldn't
be too much harder to fix for MacOS.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [PATCH 2/2] strerror: work around FreeBSD bug

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1048 bytes --]

On 06/07/2011 05:09 PM, Eric Blake wrote:
> On 06/07/2011 03:29 AM, Bruno Haible wrote:
>> Eric Blake wrote:
>>> * lib/strerror.c (strerror): Special case 0.
>>
>> Thanks.
>>
>> On MacOS X 10.5, I'm still seeing these failures:
>>
>>   test-strerror.c:63: assertion failed
>>   FAIL: test-strerror
> 
> strerror(0) misbehavior.  Hmm, while FreeBSD was setting EINVAL to make
> the failure explicit, MacOS is claiming success while still outputting
> "Unknown error: 0".  I'll fix that with a strstr() test in the .m4 files.

Oh hideous.

strerror(0) => Unknown error: 0
strerror_r(0,buf,len) => Undefined error: 0
strerror(-1) => Unknown error: -1
strerror_r(-1,buf,len) => Unknown error: -1

I don't think POSIX requires the same strings for the two
implementations, but MacOS is the first implementation where the strings
are intentionally different, but only for a single errnum value.  Fix
coming up soon.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

MacOS strerror_r [was: [PATCH 2/2] strerror: work around FreeBSD bug]

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1074 bytes --]

On 06/07/2011 09:50 PM, Eric Blake wrote:
>> strerror(0) misbehavior.  Hmm, while FreeBSD was setting EINVAL to make
>> the failure explicit, MacOS is claiming success while still outputting
>> "Unknown error: 0".  I'll fix that with a strstr() test in the .m4 files.
> 
> Oh hideous.
> 
> strerror(0) => Unknown error: 0
> strerror_r(0,buf,len) => Undefined error: 0
> strerror(-1) => Unknown error: -1
> strerror_r(-1,buf,len) => Unknown error: -1
> 
> I don't think POSIX requires the same strings for the two
> implementations, but MacOS is the first implementation where the strings
> are intentionally different, but only for a single errnum value.  Fix
> coming up soon.

I fixed that, and found the next bug:

$ ./foo 1 23
Operation not permitte(34)
$ ./foo 1 24
Operation not permitted(0)

Ouch.  MacOS is _silently_ writing a NUL byte beyond the array bounds,
if the input buffer size is exactly equal to strlen(message size).

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: MacOS strerror_r

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 565 bytes --]

On 06/08/2011 05:38 AM, Eric Blake wrote:
> I fixed that, and found the next bug:
> 
> $ ./foo 1 23
> Operation not permitte(34)
> $ ./foo 1 24
> Operation not permitted(0)
> 
> Ouch.  MacOS is _silently_ writing a NUL byte beyond the array bounds,
> if the input buffer size is exactly equal to strlen(message size).

I can't count today.  I'm seeing something fail, but after re-counting,
I'm not seeing an out-of-bounds write, after all.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [PATCH 2/2] strerror: work around FreeBSD bug

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 675 bytes --]

On 06/07/2011 09:50 PM, Eric Blake wrote:
> Oh hideous.
> 
> strerror(0) => Unknown error: 0
> strerror_r(0,buf,len) => Undefined error: 0
> strerror(-1) => Unknown error: -1
> strerror_r(-1,buf,len) => Unknown error: -1
> 
> I don't think POSIX requires the same strings for the two
> implementations,

Not directly, but C99 (and therefore POSIX) requires perror and strerror
to use the same strings, and if perror is implemented in terms of
strerror_r, then strerror and strerror_r better use the same strings.
At any rate, my patch fixes things.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]