This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA 09/23] Remove close cleanup
- From: Simon Marchi <simon dot marchi at polymtl dot ca>
- To: Tom Tromey <tom at tromey dot com>
- Cc: Pedro Alves <palves at redhat dot com>, gdb-patches at sourceware dot org
- Date: Mon, 31 Jul 2017 21:08:26 +0200
- Subject: Re: [RFA 09/23] Remove close cleanup
- Authentication-results: sourceware.org; auth=none
- References: <20170503224626.2818-1-tom@tromey.com> <20170503224626.2818-10-tom@tromey.com> <ca2efb7d-21f7-f935-fbe3-86d09916d0f0@redhat.com> <87iniom59v.fsf@tromey.com>
On 2017-07-20 00:27, Tom Tromey wrote:
"Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> Patch looks good.
Pedro> The bit below made me stop for a second:
[...]
Pedro> Would the following be too clever?
Pedro> /* Restore errno on exit to the value saved by the
Pedro> last save() call. Ctor saves. */
Pedro> struct scoped_errno_restore
[...]
Pedro> /* Save errno and return MNSH_FS_ERROR. */
Pedro> auto fs_err = [&] ()
Pedro> {
Pedro> restore_errno.save ();
Pedro> return MNSH_FS_ERROR;
Pedro> };
[...]
Pedro> [It gets rid of both the scope, and the gotos.]
How about making fd_closer a template, like:
template<int (*CLOSER) (int) = close>
class fd_closer
...
~fd_closer ()
{
if (m_fd != -1)
CLOSER (m_fd);
}
Then in linux-namespaces.c:
/* A function like "close" that saves and restores errno. */
static int
close_saving_fd (int fd)
{
scoped_restore save_errno = make_scoped_restore (&errno);
return close (fd);
}
...
gdb::fd_closer<close_saving_errno> close_fd (fd);
This also removes the scope and the gotos.
The main plus is that it's simpler. The main minus is that it's ad
hoc.
Tom
(Responding here although I am reviewing v2)
Instead of finding a solution to preserve errno, I think we should
return it explicitly when needed, it would be more robust and easier to
follow the trail where the value comes from. That would mean changing
static enum mnsh_fs_code
linux_mntns_access_fs (pid_t pid)
to
static enum mnsh_fs_code
linux_mntns_access_fs (pid_t pid, int *errnop)
and possibly others. In that case, *ERRNOP would be set if
MNSH_FS_ERROR is returned.
Simon