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

[Andrew Cooper <andrew.cooper3@citrix.com>]

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@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Charles Arnold <carnold@suse.com>
> ---
>  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;
>  }