* [PATCH 1/5] libxl, gc: fix memory leak in libxl__strndup()
@ 2014-03-11 14:15 Lai Jiangshan
2014-03-11 14:15 ` [PATCH 2/5] libxl: simplify libxl__dirname() Lai Jiangshan
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Lai Jiangshan @ 2014-03-11 14:15 UTC (permalink / raw)
To: xen-devel; +Cc: Keir Fraser, Ian Jackson, Ian Campbell, Lai Jiangshan
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
tools/libxl/libxl_internal.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
index cf17658..16b82b9 100644
--- a/tools/libxl/libxl_internal.c
+++ b/tools/libxl/libxl_internal.c
@@ -155,6 +155,8 @@ char *libxl__strndup(libxl__gc *gc, const char *c, size_t n)
if (!s) libxl__alloc_failed(CTX, __func__, n, 1);
+ libxl__ptr_add(gc, s);
+
return s;
}
--
1.7.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/5] libxl: simplify libxl__dirname()
2014-03-11 14:15 [PATCH 1/5] libxl, gc: fix memory leak in libxl__strndup() Lai Jiangshan
@ 2014-03-11 14:15 ` Lai Jiangshan
2014-03-13 16:51 ` Ian Jackson
2014-03-11 14:15 ` [PATCH 3/5] libxl: add @count to libxl_gc Lai Jiangshan
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Lai Jiangshan @ 2014-03-11 14:15 UTC (permalink / raw)
To: xen-devel; +Cc: Keir Fraser, Ian Jackson, Ian Campbell, Lai Jiangshan
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
tools/libxl/libxl_internal.c | 8 +++-----
1 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
index 16b82b9..1db48b6 100644
--- a/tools/libxl/libxl_internal.c
+++ b/tools/libxl/libxl_internal.c
@@ -162,14 +162,12 @@ char *libxl__strndup(libxl__gc *gc, const char *c, size_t n)
char *libxl__dirname(libxl__gc *gc, const char *s)
{
- char *c;
- char *ptr = libxl__strdup(gc, s);
+ char *c = strrchr(s, '/');
- c = strrchr(ptr, '/');
if (!c)
return NULL;
- *c = '\0';
- return ptr;
+
+ return libxl__strndup(gc, s, c - s);
}
void libxl__logv(libxl_ctx *ctx, xentoollog_level msglevel, int errnoval,
--
1.7.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/5] libxl: add @count to libxl_gc
2014-03-11 14:15 [PATCH 1/5] libxl, gc: fix memory leak in libxl__strndup() Lai Jiangshan
2014-03-11 14:15 ` [PATCH 2/5] libxl: simplify libxl__dirname() Lai Jiangshan
@ 2014-03-11 14:15 ` Lai Jiangshan
2014-03-13 16:52 ` Ian Jackson
2014-03-11 14:15 ` [PATCH 4/5] xc_save: avoid to alloc local constant string Lai Jiangshan
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Lai Jiangshan @ 2014-03-11 14:15 UTC (permalink / raw)
To: xen-devel; +Cc: Keir Fraser, Ian Jackson, Ian Campbell, Lai Jiangshan
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
tools/libxl/libxl_internal.c | 22 ++++++++--------------
tools/libxl/libxl_internal.h | 2 ++
2 files changed, 10 insertions(+), 14 deletions(-)
diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
index 1db48b6..a79d841 100644
--- a/tools/libxl/libxl_internal.c
+++ b/tools/libxl/libxl_internal.c
@@ -32,8 +32,6 @@ void libxl__alloc_failed(libxl_ctx *ctx, const char *func,
void libxl__ptr_add(libxl__gc *gc, void *ptr)
{
- int i;
-
if (!libxl__gc_is_real(gc))
return;
@@ -41,22 +39,17 @@ void libxl__ptr_add(libxl__gc *gc, void *ptr)
return;
/* fast case: we have space in the array for storing the pointer */
- for (i = 0; i < gc->alloc_maxsize; i++) {
- if (!gc->alloc_ptrs[i]) {
- gc->alloc_ptrs[i] = ptr;
- return;
- }
- }
+ if (gc->count < gc->alloc_maxsize)
+ gc->alloc_ptrs[gc->count++] = ptr;
+
int new_maxsize = gc->alloc_maxsize * 2 + 25;
assert(new_maxsize < INT_MAX / sizeof(void*) / 2);
gc->alloc_ptrs = realloc(gc->alloc_ptrs, new_maxsize * sizeof(void *));
if (!gc->alloc_ptrs)
libxl__alloc_failed(CTX, __func__, new_maxsize, sizeof(void*));
- gc->alloc_ptrs[gc->alloc_maxsize++] = ptr;
-
- while (gc->alloc_maxsize < new_maxsize)
- gc->alloc_ptrs[gc->alloc_maxsize++] = 0;
+ gc->alloc_ptrs[gc->count++] = ptr;
+ gc->alloc_maxsize = new_maxsize;
return;
}
@@ -68,7 +61,7 @@ void libxl__free_all(libxl__gc *gc)
assert(libxl__gc_is_real(gc));
- for (i = 0; i < gc->alloc_maxsize; i++) {
+ for (i = 0; i < gc->count; i++) {
ptr = gc->alloc_ptrs[i];
gc->alloc_ptrs[i] = NULL;
free(ptr);
@@ -76,6 +69,7 @@ void libxl__free_all(libxl__gc *gc)
free(gc->alloc_ptrs);
gc->alloc_ptrs = 0;
gc->alloc_maxsize = 0;
+ gc->count = 0;
}
void *libxl__zalloc(libxl__gc *gc, int bytes)
@@ -107,7 +101,7 @@ void *libxl__realloc(libxl__gc *gc, void *ptr, size_t new_size)
if (ptr == NULL) {
libxl__ptr_add(gc, new_ptr);
} else if (new_ptr != ptr && libxl__gc_is_real(gc)) {
- for (i = 0; i < gc->alloc_maxsize; i++) {
+ for (i = 0; i < gc->count; i++) {
if (gc->alloc_ptrs[i] == ptr) {
gc->alloc_ptrs[i] = new_ptr;
break;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 1bd23ff..d2100d8 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -294,6 +294,7 @@ struct libxl__poller {
struct libxl__gc {
/* mini-GC */
+ int count;
int alloc_maxsize; /* -1 means this is the dummy non-gc gc */
void **alloc_ptrs;
libxl_ctx *owner;
@@ -438,6 +439,7 @@ struct libxl__ao {
};
#define LIBXL_INIT_GC(gc,ctx) do{ \
+ (gc).count = 0; \
(gc).alloc_maxsize = 0; \
(gc).alloc_ptrs = 0; \
(gc).owner = (ctx); \
--
1.7.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 4/5] xc_save: avoid to alloc local constant string
2014-03-11 14:15 [PATCH 1/5] libxl, gc: fix memory leak in libxl__strndup() Lai Jiangshan
2014-03-11 14:15 ` [PATCH 2/5] libxl: simplify libxl__dirname() Lai Jiangshan
2014-03-11 14:15 ` [PATCH 3/5] libxl: add @count to libxl_gc Lai Jiangshan
@ 2014-03-11 14:15 ` Lai Jiangshan
2014-03-13 16:55 ` Ian Jackson
2014-03-11 14:15 ` [PATCH 5/5] xc_save: simplify switch_qemu_logdirty() by use stack strings Lai Jiangshan
2014-03-13 16:50 ` [PATCH 1/5] libxl, gc: fix memory leak in libxl__strndup() Ian Jackson
4 siblings, 1 reply; 11+ messages in thread
From: Lai Jiangshan @ 2014-03-11 14:15 UTC (permalink / raw)
To: xen-devel; +Cc: Keir Fraser, Ian Jackson, Ian Campbell, Lai Jiangshan
cmd_str will only cleanup by other place,
it don't need to be allocated.
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
tools/xcutils/xc_save.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/tools/xcutils/xc_save.c b/tools/xcutils/xc_save.c
index e34bd2c..1304433 100644
--- a/tools/xcutils/xc_save.c
+++ b/tools/xcutils/xc_save.c
@@ -128,8 +128,7 @@ static int switch_qemu_logdirty(int domid, unsigned int enable, void *data)
if (!xs_watch(xs, path, "qemu-logdirty-ret"))
errx(1, "can't set watch in store (%s)\n", path);
- if (!(cmd_str = strdup( enable == 0 ? "disable" : "enable")))
- errx(1, "can't get logdirty cmd path in store");
+ cmd_str = enable == 0 ? "disable" : "enable";
/* Tell qemu that we want it to start logging dirty page to Xen */
strcpy(p, "cmd");
@@ -157,7 +156,6 @@ static int switch_qemu_logdirty(int domid, unsigned int enable, void *data)
goto read_again;
free(path);
- free(cmd_str);
free(ret_str);
return 0;
--
1.7.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 5/5] xc_save: simplify switch_qemu_logdirty() by use stack strings
2014-03-11 14:15 [PATCH 1/5] libxl, gc: fix memory leak in libxl__strndup() Lai Jiangshan
` (2 preceding siblings ...)
2014-03-11 14:15 ` [PATCH 4/5] xc_save: avoid to alloc local constant string Lai Jiangshan
@ 2014-03-11 14:15 ` Lai Jiangshan
2014-03-13 16:54 ` Ian Jackson
2014-03-13 16:50 ` [PATCH 1/5] libxl, gc: fix memory leak in libxl__strndup() Ian Jackson
4 siblings, 1 reply; 11+ messages in thread
From: Lai Jiangshan @ 2014-03-11 14:15 UTC (permalink / raw)
To: xen-devel; +Cc: Keir Fraser, Ian Jackson, Ian Campbell, Lai Jiangshan
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
tools/xcutils/xc_save.c | 35 +++++++++++++----------------------
1 files changed, 13 insertions(+), 22 deletions(-)
diff --git a/tools/xcutils/xc_save.c b/tools/xcutils/xc_save.c
index 1304433..521bbae 100644
--- a/tools/xcutils/xc_save.c
+++ b/tools/xcutils/xc_save.c
@@ -101,40 +101,33 @@ static int suspend(void* data)
* the new active buffer. xc_save can then process and clear the old
* active buffer. */
-
static int switch_qemu_logdirty(int domid, unsigned int enable, void *data)
{
struct xs_handle *xs;
- char *path, *p, *ret_str, *cmd_str, **watch;
+ char *ret_str, *cmd_str, **watch;
unsigned int len;
struct timeval tv;
fd_set fdset;
+#define PATH_FMT "/local/domain/0/device-model/%d/logdirty/%s"
+#define PATH_LEN (sizeof(PATH_FMT) + 10 + sizeof("cmd"))
+ char path_cmd[PATH_LEN], path_ret[PATH_LEN];
+
+ snprintf(path_cmd, PATH_LEN, PATH_FMT, domid, "cmd");
+ snprintf(path_ret, PATH_LEN, PATH_FMT, domid, "ret");
+
if ((xs = xs_daemon_open()) == NULL)
errx(1, "Couldn't contact xenstore");
- if (!(path = strdup("/local/domain/0/device-model/")))
- errx(1, "can't get domain path in store");
- if (!(path = realloc(path, strlen(path)
- + 10
- + strlen("/logdirty/cmd") + 1)))
- errx(1, "no memory for constructing xenstore path");
- snprintf(path + strlen(path), 11, "%i", domid);
- strcat(path, "/logdirty/");
- p = path + strlen(path);
-
/* Watch for qemu's return value */
- strcpy(p, "ret");
- if (!xs_watch(xs, path, "qemu-logdirty-ret"))
- errx(1, "can't set watch in store (%s)\n", path);
+ if (!xs_watch(xs, path_ret, "qemu-logdirty-ret"))
+ errx(1, "can't set watch in store (%s)\n", path_ret);
cmd_str = enable == 0 ? "disable" : "enable";
/* Tell qemu that we want it to start logging dirty page to Xen */
- strcpy(p, "cmd");
- if (!xs_write(xs, XBT_NULL, path, cmd_str, strlen(cmd_str)))
- errx(1, "can't write to store path (%s)\n",
- path);
+ if (!xs_write(xs, XBT_NULL, path_cmd, cmd_str, strlen(cmd_str)))
+ errx(1, "can't write to store path (%s)\n", path_cmd);
/* Wait a while for qemu to signal that it has service logdirty command */
read_again:
@@ -149,13 +142,11 @@ static int switch_qemu_logdirty(int domid, unsigned int enable, void *data)
watch = xs_read_watch(xs, &len);
free(watch);
- strcpy(p, "ret");
- ret_str = xs_read(xs, XBT_NULL, path, &len);
+ ret_str = xs_read(xs, XBT_NULL, path_ret, &len);
if (ret_str == NULL || strcmp(ret_str, cmd_str))
/* Watch fired but value is not yet right */
goto read_again;
- free(path);
free(ret_str);
return 0;
--
1.7.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/5] libxl, gc: fix memory leak in libxl__strndup()
2014-03-11 14:15 [PATCH 1/5] libxl, gc: fix memory leak in libxl__strndup() Lai Jiangshan
` (3 preceding siblings ...)
2014-03-11 14:15 ` [PATCH 5/5] xc_save: simplify switch_qemu_logdirty() by use stack strings Lai Jiangshan
@ 2014-03-13 16:50 ` Ian Jackson
4 siblings, 0 replies; 11+ messages in thread
From: Ian Jackson @ 2014-03-13 16:50 UTC (permalink / raw)
To: Lai Jiangshan; +Cc: Keir Fraser, Ian Campbell, xen-devel
Lai Jiangshan writes ("[PATCH 1/5] libxl,gc: fix memory leak in libxl__strndup()"):
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/5] libxl: simplify libxl__dirname()
2014-03-11 14:15 ` [PATCH 2/5] libxl: simplify libxl__dirname() Lai Jiangshan
@ 2014-03-13 16:51 ` Ian Jackson
0 siblings, 0 replies; 11+ messages in thread
From: Ian Jackson @ 2014-03-13 16:51 UTC (permalink / raw)
To: Lai Jiangshan; +Cc: Keir Fraser, Ian Campbell, xen-devel
Lai Jiangshan writes ("[PATCH 2/5] libxl: simplify libxl__dirname()"):
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/5] libxl: add @count to libxl_gc
2014-03-11 14:15 ` [PATCH 3/5] libxl: add @count to libxl_gc Lai Jiangshan
@ 2014-03-13 16:52 ` Ian Jackson
0 siblings, 0 replies; 11+ messages in thread
From: Ian Jackson @ 2014-03-13 16:52 UTC (permalink / raw)
To: Lai Jiangshan; +Cc: Keir Fraser, Ian Campbell, xen-devel
Lai Jiangshan writes ("[PATCH 3/5] libxl: add @count to libxl_gc"):
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
I'm afraid this needs a lot more rationale.
Ian.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 5/5] xc_save: simplify switch_qemu_logdirty() by use stack strings
2014-03-11 14:15 ` [PATCH 5/5] xc_save: simplify switch_qemu_logdirty() by use stack strings Lai Jiangshan
@ 2014-03-13 16:54 ` Ian Jackson
2014-03-14 9:21 ` Lai Jiangshan
0 siblings, 1 reply; 11+ messages in thread
From: Ian Jackson @ 2014-03-13 16:54 UTC (permalink / raw)
To: Lai Jiangshan; +Cc: Keir Fraser, Ian Campbell, xen-devel
Lai Jiangshan writes ("[PATCH 5/5] xc_save: simplify switch_qemu_logdirty() by use stack strings"):
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
Thanks for this submission. This is quite fiddly to review. Can you
explain what the concrete benefit of this is ?
Thanks,
Ian.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 4/5] xc_save: avoid to alloc local constant string
2014-03-11 14:15 ` [PATCH 4/5] xc_save: avoid to alloc local constant string Lai Jiangshan
@ 2014-03-13 16:55 ` Ian Jackson
0 siblings, 0 replies; 11+ messages in thread
From: Ian Jackson @ 2014-03-13 16:55 UTC (permalink / raw)
To: Lai Jiangshan; +Cc: Keir Fraser, Ian Campbell, xen-devel
Lai Jiangshan writes ("[PATCH 4/5] xc_save: avoid to alloc local constant string"):
> cmd_str will only cleanup by other place,
> it don't need to be allocated.
>
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 5/5] xc_save: simplify switch_qemu_logdirty() by use stack strings
2014-03-13 16:54 ` Ian Jackson
@ 2014-03-14 9:21 ` Lai Jiangshan
0 siblings, 0 replies; 11+ messages in thread
From: Lai Jiangshan @ 2014-03-14 9:21 UTC (permalink / raw)
To: Ian Jackson; +Cc: Keir Fraser, Ian Campbell, xen-devel
On 03/14/2014 12:54 AM, Ian Jackson wrote:
> Lai Jiangshan writes ("[PATCH 5/5] xc_save: simplify switch_qemu_logdirty() by use stack strings"):
>> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
>
> Thanks for this submission. This is quite fiddly to review. Can you
> explain what the concrete benefit of this is ?
>
Hi, Ian Jackson.
no concrete benefit, but help for reviewing:
simplify by "allocating" strings from stack.
reduce memory-allocation and reduce error-handling-branch.
path_cmd and path_ret strings are separated.
Thanks,
Lai
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2014-03-14 9:21 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-11 14:15 [PATCH 1/5] libxl, gc: fix memory leak in libxl__strndup() Lai Jiangshan
2014-03-11 14:15 ` [PATCH 2/5] libxl: simplify libxl__dirname() Lai Jiangshan
2014-03-13 16:51 ` Ian Jackson
2014-03-11 14:15 ` [PATCH 3/5] libxl: add @count to libxl_gc Lai Jiangshan
2014-03-13 16:52 ` Ian Jackson
2014-03-11 14:15 ` [PATCH 4/5] xc_save: avoid to alloc local constant string Lai Jiangshan
2014-03-13 16:55 ` Ian Jackson
2014-03-11 14:15 ` [PATCH 5/5] xc_save: simplify switch_qemu_logdirty() by use stack strings Lai Jiangshan
2014-03-13 16:54 ` Ian Jackson
2014-03-14 9:21 ` Lai Jiangshan
2014-03-13 16:50 ` [PATCH 1/5] libxl, gc: fix memory leak in libxl__strndup() Ian Jackson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.