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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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.

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

~Andrew

>       }
>       return 1;
>  }


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