From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 4/4] libxenstat: qmp_read fix and cleanup Date: Wed, 8 Apr 2015 16:25:06 +0100 Message-ID: <55254852.4010204@citrix.com> References: <1428505275-6563-1-git-send-email-wei.liu2@citrix.com> <1428505275-6563-5-git-send-email-wei.liu2@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1428505275-6563-5-git-send-email-wei.liu2@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Wei Liu , xen-devel@lists.xen.org Cc: Charles Arnold , Ian Jackson , Ian Campbell List-Id: xen-devel@lists.xenproject.org On 08/04/15 16:01, Wei Liu wrote: > The second argument of poll(2) is the number of file descriptors. POLLIN > is defined as 1 so it happens to work. > > Also do two cleanups while I was there: > 1. There is only one fd, so a one-element array is enough. > 2. Initialise pfd to make code linter happy. > > Signed-off-by: Wei Liu > Cc: Ian Campbell > Cc: Ian Jackson > Cc: Charles Arnold > --- > tools/xenstat/libxenstat/src/xenstat_qmp.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/tools/xenstat/libxenstat/src/xenstat_qmp.c b/tools/xenstat/libxenstat/src/xenstat_qmp.c > index e4f343b..260cd34 100644 > --- a/tools/xenstat/libxenstat/src/xenstat_qmp.c > +++ b/tools/xenstat/libxenstat/src/xenstat_qmp.c > @@ -289,13 +289,14 @@ static size_t qmp_write(int qfd, char *cmd, size_t cmd_len) > static int qmp_read(int qfd, unsigned char **qstats) > { > unsigned char buf[1024], *ptr; > - struct pollfd pfd[2]; > + struct pollfd pfd[1]; > int n, qsize = 0; > > *qstats = NULL; > + memset(pfd, 0, sizeof(pfd)); > pfd[0].fd = qfd; > pfd[0].events = POLLIN; > - while ((n = poll(pfd, POLLIN, 10)) > 0) { > + while ((n = poll(pfd, 1, 10)) > 0) { > if (pfd[0].revents & POLLIN) { > if ((n = read(qfd, buf, sizeof(buf))) < 0) { > free(*qstats); > @@ -311,6 +312,9 @@ static int qmp_read(int qfd, unsigned char **qstats) > ptr[qsize] = 0; > *qstats = ptr; > } > + memset(pfd, 0, sizeof(pfd)); > + pfd[0].fd = qfd; > + pfd[0].events = POLLIN; I don't think this is necessary. .fd and .events should be untouched by poll(). Ignore the complaint about revents being uninitialised. That is because Coverity doesn't have a model for poll(), and doesn't know that poll() writes to revents. I was going to find some copious free time to correctly model poll(), which will fix several related defects. ~Andrew > } > return 1; > }