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

[Wenchao Xia <xiawenc@linux.vnet.ibm.com>]

于 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 <xiawenc@linux.vnet.ibm.com>
>> ---
>
>> +++ 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