This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 3/7] Introduce nat/linux-namespaces.[ch]
- From: Pedro Alves <palves at redhat dot com>
- To: Gary Benson <gbenson at redhat dot com>, gdb-patches at sourceware dot org
- Date: Fri, 17 Apr 2015 15:52:45 +0100
- Subject: Re: [PATCH 3/7] Introduce nat/linux-namespaces.[ch]
- Authentication-results: sourceware.org; auth=none
- References: <1429186791-6867-1-git-send-email-gbenson at redhat dot com> <1429186791-6867-4-git-send-email-gbenson 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