This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 2/9 v2] Introduce nat/linux-namespaces.[ch]
- From: Gary Benson <gbenson at redhat dot com>
- To: Pedro Alves <palves at redhat dot com>
- Cc: gdb-patches at sourceware dot org, Eli Zaretskii <eliz at gnu dot org>, Doug Evans <dje at google dot com>, Iago López Galeiras <iago at endocode dot com>
- Date: Wed, 27 May 2015 11:14:08 +0100
- Subject: Re: [PATCH 2/9 v2] Introduce nat/linux-namespaces.[ch]
- Authentication-results: sourceware.org; auth=none
- References: <1429186791-6867-1-git-send-email-gbenson at redhat dot com> <1430395542-16017-3-git-send-email-gbenson at redhat dot com> <555DF224 dot 8020101 at redhat dot com>
Pedro Alves wrote:
> On 04/30/2015 01:05 PM, Gary Benson wrote:
> ...
> > + For avoidance of doubt, if the helper process receives a
> > + message it doesn't handle it will reply with MNSH_MSG_ERROR.
> > + If the main process receives MNSH_MSG_ERROR at any time then
> > + it will call internal_error. If internal_error causes the
> > + main process to exit, the helper will notice this and also
> > + exit. The helper will not exit until the main process
> > + terminates, so if the user continues through internal_error
> > + the helper will still be there awaiting requests from the
> > + main process.
> ...
> > +/* Mount namespace helper message types. */
> > +
> > +enum mnsh_msg_type
> > + {
> > + /* An unrecoverable communication error occurred.
>
> I think "unrecoverable" here sounds a bit confusing, as it
> contradicts the comment above that explains that the helper
> is still awaiting requests if the user decides to continue
> after internal_error.
Ok, I'll remove the "unrecoverable" and leave the rest of the
message alone.
> > + /* A request that the helper call readlink. The single
> > + argument (the filename) should be passed in BUF, and
> > + should include a terminating NUL character. The helper
>
> Missing double-space.
Ok.
> > +static void
> > +mnsh_main (int sock)
> > +{
> > + while (1)
> > + {
> > + enum mnsh_msg_type type;
> > + int fd, int1, int2;
> > + char buf[PATH_MAX];
> > + ssize_t size, response = -1;
> > +
> > + size = mnsh_recv_message (sock, &type,
> > + &fd, &int1, &int2,
> > + buf, sizeof (buf));
> > +
> > + if (size >= 0 && size < sizeof (buf))
> > + {
> > + switch (type)
> > + {
> > + case MNSH_REQ_SETNS:
> > + if (fd > 0)
> > + response = mnsh_handle_setns (sock, fd, int1);
> > + break;
> > +
> > + case MNSH_REQ_OPEN:
> > + if (buf[size - 1] == '\0')
>
> Why these == '\0' checks? To protect against bugs? In
> that case, I guess this should be:
> if (size > 0 && buf[size - 1] == '\0')
>
> > + response = mnsh_handle_open (sock, buf, int1, int2);
> > + break;
Yeah, that's better, I'll change it.
> And that's it. Really all looks good to me. :-)
Sweet :)
Thanks,
Gary
--
http://gbenson.net/