Re: [Xen-devel] [PATCH 4/4] libxenstat: qmp_read fix and cleanup

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Wed, Apr 08, 2015 at 04:25:06PM +0100, Andrew Cooper wrote:
> 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 <wei.liu2@xxxxxxxxxx>
> > Cc: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> > Cc: Charles Arnold <carnold@xxxxxxxx>
> > ---
> >  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.
> 

Sure.

> I was going to find some copious free time to correctly model poll(),
> which will fix several related defects.
> 

That would be good!

Wei.

> ~Andrew
> 
> >     }
> >     return 1;
> >  }

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel