* [PATCH v2 1/4] libxenstat: reuse xc_handle open in xenstat_init
2015-04-08 16:08 [PATCH v2 0/4] libxenstat bug fixes and cleanups Wei Liu
@ 2015-04-08 16:08 ` Wei Liu
2015-04-08 16:11 ` Andrew Cooper
2015-04-08 16:08 ` [PATCH v2 2/4] libxenstat: YAJL_GET_STRING may return NULL Wei Liu
` (3 subsequent siblings)
4 siblings, 1 reply; 8+ messages in thread
From: Wei Liu @ 2015-04-08 16:08 UTC (permalink / raw)
To: xen-devel
Cc: Ian Jackson, andrew.cooper3, Wei Liu, Ian Campbell, Charles Arnold
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 | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/tools/xenstat/libxenstat/src/xenstat_qmp.c b/tools/xenstat/libxenstat/src/xenstat_qmp.c
index 2cb99e9..10ae104 100644
--- a/tools/xenstat/libxenstat/src/xenstat_qmp.c
+++ b/tools/xenstat/libxenstat/src/xenstat_qmp.c
@@ -357,17 +357,14 @@ static int qmp_connect(char *path)
}
/* Get up to 1024 active domains */
-static xc_domaininfo_t *get_domain_ids(int *num_doms)
+static xc_domaininfo_t *get_domain_ids(xc_interface *xc_handle, int *num_doms)
{
xc_domaininfo_t *dominfo;
- xc_interface *xc_handle;
dominfo = calloc(1024, sizeof(xc_domaininfo_t));
if (dominfo == NULL)
return NULL;
- xc_handle = xc_interface_open(0,0,0);
*num_doms = xc_domain_getinfolist(xc_handle, 0, 1024, dominfo);
- xc_interface_close(xc_handle);
return dominfo;
}
@@ -406,7 +403,7 @@ void read_attributes_qdisk(xenstat_node * node)
char path[80];
int i, qfd, num_doms;
- dominfo = get_domain_ids(&num_doms);
+ dominfo = get_domain_ids(node->handle->xc_handle, &num_doms);
if (dominfo == NULL)
return;
--
1.9.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/4] libxenstat: reuse xc_handle open in xenstat_init
2015-04-08 16:08 ` [PATCH v2 1/4] libxenstat: reuse xc_handle open in xenstat_init Wei Liu
@ 2015-04-08 16:11 ` Andrew Cooper
0 siblings, 0 replies; 8+ messages in thread
From: Andrew Cooper @ 2015-04-08 16:11 UTC (permalink / raw)
To: Wei Liu, xen-devel; +Cc: Charles Arnold, Ian Jackson, Ian Campbell
On 08/04/15 17:08, Wei Liu wrote:
> 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>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> tools/xenstat/libxenstat/src/xenstat_qmp.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/tools/xenstat/libxenstat/src/xenstat_qmp.c b/tools/xenstat/libxenstat/src/xenstat_qmp.c
> index 2cb99e9..10ae104 100644
> --- a/tools/xenstat/libxenstat/src/xenstat_qmp.c
> +++ b/tools/xenstat/libxenstat/src/xenstat_qmp.c
> @@ -357,17 +357,14 @@ static int qmp_connect(char *path)
> }
>
> /* Get up to 1024 active domains */
> -static xc_domaininfo_t *get_domain_ids(int *num_doms)
> +static xc_domaininfo_t *get_domain_ids(xc_interface *xc_handle, int *num_doms)
> {
> xc_domaininfo_t *dominfo;
> - xc_interface *xc_handle;
>
> dominfo = calloc(1024, sizeof(xc_domaininfo_t));
> if (dominfo == NULL)
> return NULL;
> - xc_handle = xc_interface_open(0,0,0);
> *num_doms = xc_domain_getinfolist(xc_handle, 0, 1024, dominfo);
> - xc_interface_close(xc_handle);
> return dominfo;
> }
>
> @@ -406,7 +403,7 @@ void read_attributes_qdisk(xenstat_node * node)
> char path[80];
> int i, qfd, num_doms;
>
> - dominfo = get_domain_ids(&num_doms);
> + dominfo = get_domain_ids(node->handle->xc_handle, &num_doms);
> if (dominfo == NULL)
> return;
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 2/4] libxenstat: YAJL_GET_STRING may return NULL
2015-04-08 16:08 [PATCH v2 0/4] libxenstat bug fixes and cleanups Wei Liu
2015-04-08 16:08 ` [PATCH v2 1/4] libxenstat: reuse xc_handle open in xenstat_init Wei Liu
@ 2015-04-08 16:08 ` Wei Liu
2015-04-08 16:08 ` [PATCH v2 3/4] libxenstat: always free qmp_stats Wei Liu
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Wei Liu @ 2015-04-08 16:08 UTC (permalink / raw)
To: xen-devel
Cc: Ian Jackson, andrew.cooper3, Wei Liu, Ian Campbell, Charles Arnold
Passing NULL to strcmp can cause segmentation fault. Continue in that
case.
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>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
tools/xenstat/libxenstat/src/xenstat_qmp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/xenstat/libxenstat/src/xenstat_qmp.c b/tools/xenstat/libxenstat/src/xenstat_qmp.c
index 10ae104..aad15c8 100644
--- a/tools/xenstat/libxenstat/src/xenstat_qmp.c
+++ b/tools/xenstat/libxenstat/src/xenstat_qmp.c
@@ -110,7 +110,7 @@ static char *qmp_get_block_image(xenstat_node *node, char *qmp_devname, int qfd)
ptr[0] = qblock[QMP_BLOCK_DEVICE]; /* "device" */
if ((dev_obj = yajl_tree_get(n, ptr, yajl_t_any)) != NULL) {
tmp = YAJL_GET_STRING(dev_obj);
- if (strcmp(qmp_devname, tmp))
+ if (!tmp || strcmp(qmp_devname, tmp))
continue;
}
else
--
1.9.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 3/4] libxenstat: always free qmp_stats
2015-04-08 16:08 [PATCH v2 0/4] libxenstat bug fixes and cleanups Wei Liu
2015-04-08 16:08 ` [PATCH v2 1/4] libxenstat: reuse xc_handle open in xenstat_init Wei Liu
2015-04-08 16:08 ` [PATCH v2 2/4] libxenstat: YAJL_GET_STRING may return NULL Wei Liu
@ 2015-04-08 16:08 ` Wei Liu
2015-04-08 16:08 ` [PATCH v2 4/4] libxenstat: qmp_read fix and cleanup Wei Liu
2015-04-15 16:27 ` [PATCH v2 0/4] libxenstat bug fixes and cleanups Ian Campbell
4 siblings, 0 replies; 8+ messages in thread
From: Wei Liu @ 2015-04-08 16:08 UTC (permalink / raw)
To: xen-devel
Cc: Ian Jackson, andrew.cooper3, Wei Liu, Ian Campbell, Charles Arnold
Originally qmp_stats is only freed in failure path and leaked in success
path.
Instead of wiring up the success path, rearrange the code a bit to
always free qmp_stats before checking if info is NULL.
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>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
tools/xenstat/libxenstat/src/xenstat_qmp.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/xenstat/libxenstat/src/xenstat_qmp.c b/tools/xenstat/libxenstat/src/xenstat_qmp.c
index aad15c8..8d91fef 100644
--- a/tools/xenstat/libxenstat/src/xenstat_qmp.c
+++ b/tools/xenstat/libxenstat/src/xenstat_qmp.c
@@ -95,10 +95,10 @@ static char *qmp_get_block_image(xenstat_node *node, char *qmp_devname, int qfd)
return NULL;
/* Use libyajl version 2.0.3 or newer for the tree parser feature with bug fixes */
- if ((info = yajl_tree_parse((char *)qmp_stats, NULL, 0)) == NULL) {
- free(qmp_stats);
+ info = yajl_tree_parse((char *)qmp_stats, NULL, 0);
+ free(qmp_stats);
+ if (info == NULL)
return NULL;
- }
ptr[0] = qblock[QMP_BLOCK_RETURN]; /* "return" */
if ((ret_obj = yajl_tree_get(info, ptr, yajl_t_array)) == NULL)
--
1.9.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 4/4] libxenstat: qmp_read fix and cleanup
2015-04-08 16:08 [PATCH v2 0/4] libxenstat bug fixes and cleanups Wei Liu
` (2 preceding siblings ...)
2015-04-08 16:08 ` [PATCH v2 3/4] libxenstat: always free qmp_stats Wei Liu
@ 2015-04-08 16:08 ` Wei Liu
2015-04-08 16:11 ` Andrew Cooper
2015-04-15 16:27 ` [PATCH v2 0/4] libxenstat bug fixes and cleanups Ian Campbell
4 siblings, 1 reply; 8+ messages in thread
From: Wei Liu @ 2015-04-08 16:08 UTC (permalink / raw)
To: xen-devel
Cc: Ian Jackson, andrew.cooper3, Wei Liu, Ian Campbell, Charles Arnold
The second argument of poll(2) is the number of file descriptors. POLLIN
is defined as 1 so it happens to work. Also reduce the size of array to
one as there is only one file descriptor.
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 | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/xenstat/libxenstat/src/xenstat_qmp.c b/tools/xenstat/libxenstat/src/xenstat_qmp.c
index 8d91fef..5e261af 100644
--- a/tools/xenstat/libxenstat/src/xenstat_qmp.c
+++ b/tools/xenstat/libxenstat/src/xenstat_qmp.c
@@ -289,13 +289,13 @@ 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;
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);
--
1.9.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 4/4] libxenstat: qmp_read fix and cleanup
2015-04-08 16:08 ` [PATCH v2 4/4] libxenstat: qmp_read fix and cleanup Wei Liu
@ 2015-04-08 16:11 ` Andrew Cooper
0 siblings, 0 replies; 8+ messages in thread
From: Andrew Cooper @ 2015-04-08 16:11 UTC (permalink / raw)
To: Wei Liu, xen-devel; +Cc: Charles Arnold, Ian Jackson, Ian Campbell
On 08/04/15 17:08, 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 reduce the size of array to
> one as there is only one file descriptor.
>
> 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>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> tools/xenstat/libxenstat/src/xenstat_qmp.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/xenstat/libxenstat/src/xenstat_qmp.c b/tools/xenstat/libxenstat/src/xenstat_qmp.c
> index 8d91fef..5e261af 100644
> --- a/tools/xenstat/libxenstat/src/xenstat_qmp.c
> +++ b/tools/xenstat/libxenstat/src/xenstat_qmp.c
> @@ -289,13 +289,13 @@ 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;
> 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);
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 0/4] libxenstat bug fixes and cleanups
2015-04-08 16:08 [PATCH v2 0/4] libxenstat bug fixes and cleanups Wei Liu
` (3 preceding siblings ...)
2015-04-08 16:08 ` [PATCH v2 4/4] libxenstat: qmp_read fix and cleanup Wei Liu
@ 2015-04-15 16:27 ` Ian Campbell
4 siblings, 0 replies; 8+ messages in thread
From: Ian Campbell @ 2015-04-15 16:27 UTC (permalink / raw)
To: Wei Liu; +Cc: Charles Arnold, andrew.cooper3, xen-devel
On Wed, 2015-04-08 at 17:08 +0100, Wei Liu wrote:
> Wei Liu (4):
> libxenstat: reuse xc_handle open in xenstat_init
> libxenstat: YAJL_GET_STRING may return NULL
> libxenstat: always free qmp_stats
> libxenstat: qmp_read fix and cleanup
>
> tools/xenstat/libxenstat/src/xenstat_qmp.c | 19 ++++++++-----------
> 1 file changed, 8 insertions(+), 11 deletions(-)
All acked (FWIW given I supposedly reviewed the initial code, thanks for
fixing!) and applied, thanks.
^ permalink raw reply [flat|nested] 8+ messages in thread