This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA 09/23] Remove close cleanup


Patch looks good.

The bit below made me stop for a second:

On 05/03/2017 11:46 PM, Tom Tromey wrote:
> @@ -901,64 +900,59 @@ linux_mntns_access_fs (pid_t pid)
>    if (ns == NULL)
>      return MNSH_FS_DIRECT;
>  
> -  old_chain = make_cleanup (null_cleanup, NULL);
> -
>    fd = gdb_open_cloexec (linux_ns_filename (ns, pid), O_RDONLY, 0);
>    if (fd < 0)
> -    goto error;
> -
> -  make_cleanup_close (fd);
> +    return MNSH_FS_ERROR;
>  
> -  if (fstat (fd, &sb) != 0)
> -    goto error;
> +  /* Introduce a new scope here so we can reset errno after
> +     closing.  */
> +  {
> +    gdb::fd_closer close_fd (fd);
>  

Would the following be too clever?

    /* Restore errno on exit to the value saved by the
       last save() call.  Ctor saves.  */
    struct scoped_errno_restore
    {
      scoped_errno_restore () { save (); }
      ~scoped_errno_restore () { errno = m_value; }

      /* Take a snapshot of current errno.  */
      void save () { m_value = errno; }

    private:
      int m_value;
    };

    scoped_errno_restore restore_errno;
    gdb::fd_closer close_fd (fd);

    /* Save errno and return MNSH_FS_ERROR.  */
    auto fs_err = [&] ()
    {
       restore_errno.save ();
       return MNSH_FS_ERROR;
    };

    if (fstat (fd, &sb) != 0)
      return fs_err ();

    (...)

    helper = linux_mntns_get_helper ();
    if (helper == NULL)
      return fs_err ();

    size = mnsh_send_setns (helper, fd, 0);
    if (size < 0)
      return fs_err ();

[It gets rid of both the scope, and the gotos.]

Thanks,
Pedro Alves


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]