From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60271) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VEVFP-000050-9Y for qemu-devel@nongnu.org; Tue, 27 Aug 2013 22:12:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VEVFG-0005aT-BD for qemu-devel@nongnu.org; Tue, 27 Aug 2013 22:12:39 -0400 Received: from e28smtp08.in.ibm.com ([122.248.162.8]:60628) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VEVFF-0005aE-QD for qemu-devel@nongnu.org; Tue, 27 Aug 2013 22:12:30 -0400 Received: from /spool/local by e28smtp08.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 28 Aug 2013 07:31:00 +0530 Received: from d28relay01.in.ibm.com (d28relay01.in.ibm.com [9.184.220.58]) by d28dlp02.in.ibm.com (Postfix) with ESMTP id 4109F3940053 for ; Wed, 28 Aug 2013 07:42:06 +0530 (IST) Received: from d28av04.in.ibm.com (d28av04.in.ibm.com [9.184.220.66]) by d28relay01.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r7S2DtxP31260704 for ; Wed, 28 Aug 2013 07:43:55 +0530 Received: from d28av04.in.ibm.com (localhost [127.0.0.1]) by d28av04.in.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id r7S2CFOE031975 for ; Wed, 28 Aug 2013 07:42:15 +0530 Message-ID: <521D5C4C.8000502@linux.vnet.ibm.com> Date: Wed, 28 Aug 2013 10:11:24 +0800 From: Wenchao Xia MIME-Version: 1.0 References: <1377571931-9144-1-git-send-email-xiawenc@linux.vnet.ibm.com> <1377571931-9144-2-git-send-email-xiawenc@linux.vnet.ibm.com> <521D4E44.9090705@redhat.com> In-Reply-To: <521D4E44.9090705@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH V2 1/3] qemu-iotests: add unix socket help program List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: pbonzini@redhat.com, lcapitulino@redhat.com, qemu-devel@nongnu.org, anthony@codemonkey.ws, armbru@redhat.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 >> --- > >> +++ 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