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

Pedro Alves
2015-04-17T14:52:49Z

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