Re: [RFA 09/23] Remove close cleanup

[Simon Marchi <simon.marchi@polymtl.ca>]

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