[PATCH 2/6] Don't reset errno/bfd_error on 'throw_perror_with_name'

Sergio Durigan Junior
2020-02-26T20:06:32Z

Since this hunk may be a bit controversial, I decided to split it into
a separate patch. This is going to be needed by the ptrace-error
feature; GDB will need to be able to access the value of errno even
after a call to our 'perror'-like functions.

The idea here is that we actually *want* errno to propagate between
our customized 'perror' calls. We currently have this code on
'throw_perror_with_name':

/* I understand setting these is a matter of taste. Still, some people
may clear errno but not know about bfd_error. Doing this here is not
unreasonable. */
bfd_set_error (bfd_error_no_error);
errno = 0;

git blame tells me that this piece of code is pretty old; the commit
that "introduced" it is:

commit c906108c21474dfb4ed285bcc0ac6fe02cd400cc
Author: Stan Shebs <shebs@codesourcery.com>
Date: Fri Apr 16 01:35:26 1999 +0000

Initial creation of sourceware repository

so yeah...

If we go to the POSIX specification for 'perror', it doesn't really
say anything about whether errno should be preserved or not. It does,
however, say that 'perror's messages should be the same as those
returned by 'strerror', and 'strerror' is not supposed to alter errno
if the call is successful.

Maybe when our wrapper was written it was OK to modify errno, I don't
know. But I'd like to propose that we stick to POSIX in this case.

Another small hunk is the one that saves/restores errno on gdbserver's
'perror_with_name', but this one is pretty trivial, I think.

gdb/ChangeLog:
yyyy-mm-dd Sergio Durigan Junior <sergiodj@redhat.com>

* utils.c (throw_perror_with_name): Don't reset
errno/bfd_error.

gdbserver/ChangeLog:
yyyy-mm-dd Sergio Durigan Junior <sergiodj@redhat.com>

* utils.cc (perror_with_name): Save/restore errno.
---
gdb/utils.c | 6 ------
gdbserver/utils.cc | 2 ++
2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/gdb/utils.c b/gdb/utils.c
index 0b470120a2..df8add1afa 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -595,12 +595,6 @@ throw_perror_with_name (enum errors errcode, const char *string)
{
std::string combined = perror_string (string);

- /* I understand setting these is a matter of taste. Still, some people
- may clear errno but not know about bfd_error. Doing this here is not
- unreasonable. */
- bfd_set_error (bfd_error_no_error);
- errno = 0;
-
throw_error (errcode, _("%s."), combined.c_str ());
}

diff --git a/gdbserver/utils.cc b/gdbserver/utils.cc
index d88f4ac5ca..50ebba43ca 100644
--- a/gdbserver/utils.cc
+++ b/gdbserver/utils.cc
@@ -46,6 +46,7 @@ perror_with_name (const char *string)
{
const char *err;
char *combined;
+ int saved_errno = errno;

err = safe_strerror (errno);
if (err == NULL)
@@ -56,6 +57,7 @@ perror_with_name (const char *string)
strcat (combined, ": ");
strcat (combined, err);

+ errno = saved_errno;
error ("%s.", combined);
}

--
2.24.1