Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use strerror() or strerror_r() instead of sys_errlist and sys_nerr #1830

Closed
gnu-srs opened this issue Nov 26, 2014 · 10 comments
Closed

Use strerror() or strerror_r() instead of sys_errlist and sys_nerr #1830

gnu-srs opened this issue Nov 26, 2014 · 10 comments
Assignees
Milestone

Comments

@gnu-srs
Copy link

gnu-srs commented Nov 26, 2014

Hi,

Looks like your code is still using sys_errlist and sys_nerr for error messages on non-UCLIBC systems instead of strerror(). It has been specified by POSIX.1-2001, POSIX.1-2008, C89, and C99.
Why so, e.g. even GNU/Linux marks them as obsolete even if found:
/usr/include/x86_64-linux-gnu/bits/sys_errlist.h
/* sys_errlist and sys_nerr are deprecated. Use strerror instead. */

On GNU/Hurd they are not defined, therefore fish fails to build from source there. In case you want a thread-safe version of strerror(), why not use strerror_r(), as specified by POSIX.1-2001 and POSIX.1-2008.

Seems like the Debian maintainer is not comfortable with that change non-upstream.

Thanks!

@ridiculousfish
Copy link
Member

strerror with glibc calls into gettext, which takes a lock. This can deadlock, if that lock was held during fork. See #472 for example of a hang under strerror_r.

@ridiculousfish
Copy link
Member

To fix the error on GNU/Hurd, we should probably add a check for sys_errlist in the configure script.

@zanchey
Copy link
Member

zanchey commented Nov 27, 2014

Thanks @gnu-srs. In case you are referring to my comment on Debian bug 771052, I'm not the Debian maintainer but one of the upstream committers. I see there is some confusion between async-safe and thread-safe on 771052 (see also #495 and fish's Fork and Pthread Manifesto).

On the downstream bug, Samuel Thibault notes that

It is always safe in the cases where one would be able to use sys_errlist directly, actualy: in that case the glibc source code boils down to return _sys_errlist_internal[errnum]).

although it is not clear that this is definitely async-safe.

There is lots of malloc in the glibc source for strerror, __strerror_r, and it calls gettext as well.

@zanchey
Copy link
Member

zanchey commented Nov 27, 2014

What are the options if there is no sys_errlist? strerror and hope?

@zanchey
Copy link
Member

zanchey commented Nov 27, 2014

The nginx project has also run into this problem, and have solved it by... setting up their own internal sys_errlist at the beginning of execution by calling strerror() on values from 0 to sys_nerr, the value of which is set in the configure script from sys_nerr. Wow.

@ridiculousfish
Copy link
Member

That's wild

@zanchey zanchey added this to the fish-future milestone Dec 5, 2014
@zanchey
Copy link
Member

zanchey commented Dec 6, 2014

To confirm, glibc is definitely not async-safe: __strerror_r definitely calls _ aka gettext.

Solaris doesn't have sys_nerr or sys_errlist either, and strerror calls gettext. I note the OpenSolaris maintainers have elected to proceed with the use of strerror on the grounds that the documentation does not specify either way but I don't think that is the right thing to do either.

The nginx implementation is starting to look alarmingly sane.

@zanchey
Copy link
Member

zanchey commented Dec 6, 2014

Solaris uses _sys_errs[_sys_index[errnum]] internally.

@zanchey
Copy link
Member

zanchey commented Dec 8, 2014

libiberty, which comes with GCC, does something fairly similar to ngnix:

https://github.com/gcc-mirror/gcc/blob/master/libiberty/strerror.c

@zanchey
Copy link
Member

zanchey commented Dec 23, 2014

In 147078f there is a check for sys_errlist (and _sys_errs), and if neither are available, a generic message is printed with the errno. With that commit, fish now builds on Hurd so I'm going to close this issue.

@zanchey zanchey closed this as completed Dec 23, 2014
@zanchey zanchey modified the milestones: next-minor, fish-future Dec 23, 2014
@zanchey zanchey self-assigned this Dec 23, 2014
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants