Re: [Qemu-devel] [PATCH V2 1/3] qemu-iotests: add unix socket help program

[Wenchao Xia]

于 2013-8-28 9:11, Eric Blake 写道:
> On 08/26/2013 08:52 PM, Wenchao Xia wrote:
> > This program can do a sendmsg call to transfer fd with unix
> > socket, which is not supported in python2.
> >
> > The built binary will not be deleted in clean, but it is a
> > existing issue in ./tests, which should be solved in another
> > patch.
> >
> > Signed-off-by: Wenchao Xia <address@hidden>
> > ---
>
> > +++ b/tests/qemu-iotests/socket_scm_helper.c
> > @@ -0,0 +1,119 @@
> > +/*
> > + * SCM_RIGHT with unix socket help program for test
>
> s/SCM_RIGHT/&S/
>
> > +
> > +static int send_fd(int fd, int fd_to_send, const char *data, size_t len)
> > +{
> > +    struct msghdr msg;
> > +    struct iovec iov[1];
> > +    int ret;
> > +    char control[CMSG_SPACE(sizeof(int))];
> > +    struct cmsghdr *cmsg;
> > +
> > +    if (fd < 0) {
> > +        fprintf(stderr, "Socket fd is invalid.\n");
> > +        return -1;
> > +    }
> > +    if (fd_to_send < 0) {
> > +        fprintf(stderr, "Fd to send is invalid.\n");
> > +        return -1;
> > +    }
> > +    if (data == NULL || len <= 0) {
>
> len cannot be < 0, since it is size_t (or did you mean for it to be
> ssize_t?)
>
>
> > +    if (ret < 0) {
> > +        fprintf(stderr, "Failed to send msg, reason: %s.\n", strerror(errno));
>
> Messages typically shouldn't end with '.'; especially when ending the
> message with strerror.
>
> > +    }
> > +
> > +    return ret;
> > +}
> > +
> > +/*
> > + * To make things simple, the caller need to specify:
>
> s/need/needs/
>
> > + * 1. socket fd.
> > + * 2. fd to send.
> > + * 3. msg to send.
> > + */
> > +int main(int argc, char **argv, char **envp)
> > +{
> > +    int sock, fd, ret, buflen;
> > +    const char *buf;
> > +#ifdef SOCKET_SCM_DEBUG
> > +    int i;
> > +    for (i = 0; i < argc; i++) {
> > +        fprintf(stderr, "Parameter %d: %s.\n", i, argv[i]);
>
> Another useless ending '.' (and elsewhere throughout the patch)
>
> > +    }
> > +#endif
> > +
> > +    if (argc < 4) {
> > +        fprintf(stderr,
> > +                "Invalid parameter, use it as:\n"
> > +                "%s SOCKET_FD FD_TO_SEND MSG.\n",
> > +                argv[0]);
>
> This rejects too few, but not too many, arguments.  Should the condition
> be: if (argc != 4)
>
> > +        return 1;
>
> I prefer EXIT_FAILURE over the magic number 1 (multiple instances).
>
> > +    }
> > +
> > +    errno = 0;
> > +    sock = strtol(argv[1], NULL, 10);
>
> Failure to pass in a second argument means you cannot distinguish ""
> from "0" from "0a" - all three input strings for argv[1] result in
> sock==0 without you knowing any better.  If you're going to use
> strtol(), use it correctly; if you don't care about garbage, then atoi()
> is just as (in)correct and with less typing.
    I will check error more carefully with other issues addressed in
next version. Since the build system is modified, I'd like to wait a
few days to see if more comment comes.

> > +    if (errno) {
> > +        fprintf(stderr, "Failed in strtol for socket fd, reason: %s.\n",
> > +                strerror(errno));
> > +        return 1;
> > +    }
> > +    fd = strtol(argv[2], NULL, 10);
> > +    if (errno) {
> > +        fprintf(stderr, "Failed in strtol for fd to send, reason: %s.\n",
> > +                strerror(errno));
> > +        return 1;
> > +    }
> > +
> > +    buf = argv[3];
> > +    buflen = strlen(buf);
> > +
> > +    ret = send_fd(sock, fd, buf, buflen);
> > +    if (ret < 0) {
> > +        return 1;
> > +    }
> > +    return 0;
>
> I prefer EXIT_SUCCESS over the magic number 0.
>
> > +}


--
Best Regards

Wenchao Xia