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

[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.

> +    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.

> +}
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature