Re: [PATCH 3/7] Introduce nat/linux-namespaces.[ch]

[Pedro Alves <palves at redhat dot com>]

On 04/16/2015 01:19 PM, Gary Benson wrote:

> +  path = linux_ns_path_for (pid1, type);
> +  if (stat (path, &sb) != 0)
> +    {
> +      int saved_errno;
> +
> +      if (pid1 == getpid ())
> +	{
> +	  /* Assume the kernel does not support TYPE namespaces.  */
> +	  return 1;
> +	}
> +
> +      saved_errno = errno;

Don't assume any function preserves errno.  Save this right after
stat.

> +      path = linux_ns_path_for (getpid (), type);
> +      if (stat (path, &sb) != 0)
> +	{
> +	  /* Assume the kernel does not support TYPE namespaces.  */
> +	  return 1;
> +	}
> +
> +      /* We can open our own TYPE namespace but not that for process
> +	 PID.  The process might have died, or we might not have the
> +	 right permissions (though we should be attached by this time
> +	 so this seems unlikely).  In any event, we cannot make any
> +	 decisions and must throw.  */
> +      errno = saved_errno;
> +      perror_with_name (linux_ns_path_for (pid1, type));
> +    }
> +  pid1_id = sb.st_ino;
> +
> +  /* The kernel definitely supports TYPE namespaces so we cannot
> +     make any decisions if this stat fails.  */
> +  path = linux_ns_path_for (pid2, type);
> +  if (stat (path, &sb) != 0)
> +    perror_with_name (path);
> +
> +  return sb.st_ino == pid1_id;
> +}
> +
> +/* Helper function which does the work for make_cleanup_setns.  */
> +
> +static void
> +do_setns_cleanup (void *arg)
> +{
> +  int *fd = arg;
> +
> +  if (setns (*fd, 0) != 0)
> +    internal_error (__FILE__, __LINE__,
> +		    _("unable to restore namespace: %s"),
> +		    safe_strerror (errno));

And here

  if (setns (*fd, 0) != 0)
    {
       int saved_errno = errno;

       internal_error (__FILE__, __LINE__,
	   	    _("unable to restore namespace: %s"),
		    safe_strerror (saved_errno));
    }

because _() is a function call, and you can't rely on
whether safe_strerror is called before or after.


> +/* Enter the TYPE namespace of process PID and call FUNC with the
> +   argument ARG, returning to the original TYPE namespace afterwards.
> +   If process PID has the same TYPE namespace as the current process,
> +   or if TYPE namespaces are not supported, just call FUNC with ARG.
> +   Return nonzero if FUNC was called, zero otherwise (and set ERRNO). */
> +
> +extern int linux_ns_enter (int pid, const char *type,
> +			   void (*func) (void *), void *arg);


So the function:

 #1 - enters the namespace
 #2 - calls func
 #3 - exits the namespace.

IMO, "linux_ns_ENTER" isn't a good name for that.  I'd expect that a function
called "enter" do just #1 above.  Something like "linux_ns_do",
"linux_do_in_ns", "linux_in_ns", etc., would be clearer, IMO.

Thanks,
Pedro Alves