* [PATCH] Fix libxenstore memory leak when USE_PTHREAD is not defined, V2
@ 2012-09-14 14:58 Andres Lagar-Cavilla
2012-09-14 15:10 ` Ian Campbell
0 siblings, 1 reply; 3+ messages in thread
From: Andres Lagar-Cavilla @ 2012-09-14 14:58 UTC (permalink / raw)
To: xen-devel; +Cc: ian.jackson, andres, ian.campbell
tools/xenstore/xs.c | 17 +++++++++++++----
1 files changed, 13 insertions(+), 4 deletions(-)
Redefine usage of pthread_cleanup_push and _pop, to explicitly call free for
heap objects in error paths.
By the way, set a suitable errno value for an error path that had none.
This is V2 after comments from Ian Campbell.
Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>
diff -r 588d0dc298a4 -r a83fd69dcb27 tools/xenstore/xs.c
--- a/tools/xenstore/xs.c
+++ b/tools/xenstore/xs.c
@@ -110,6 +110,11 @@ struct xs_handle {
#define read_thread_exists(h) (h->read_thr_exists)
+/* Because pthread_cleanup_p* are not available when USE_PTHREAD is
+ * disabled, use these macros which convert appropriately. */
+#define cleanup_push_heap(p) cleanup_push(free, p)
+#define cleanup_pop_heap(run, p) cleanup_pop((run))
+
static void *read_thread(void *arg);
#else /* !defined(USE_PTHREAD) */
@@ -130,6 +135,9 @@ struct xs_handle {
#define cleanup_pop(run) ((void)0)
#define read_thread_exists(h) (0)
+#define cleanup_push_heap(p) ((void)0)
+#define cleanup_pop_heap(run, p) do { if ((run)) free(p); } while(0)
+
#endif
static int read_message(struct xs_handle *h, int nonblocking);
@@ -1059,7 +1067,7 @@ static int read_message(struct xs_handle
msg = malloc(sizeof(*msg));
if (msg == NULL)
goto error;
- cleanup_push(free, msg);
+ cleanup_push_heap(msg);
if (!read_all(h->fd, &msg->hdr, sizeof(msg->hdr), nonblocking)) { /* Cancellation point */
saved_errno = errno;
goto error_freemsg;
@@ -1069,7 +1077,7 @@ static int read_message(struct xs_handle
body = msg->body = malloc(msg->hdr.len + 1);
if (body == NULL)
goto error_freemsg;
- cleanup_push(free, body);
+ cleanup_push_heap(body);
if (!read_all(h->fd, body, msg->hdr.len, 0)) { /* Cancellation point */
saved_errno = errno;
goto error_freebody;
@@ -1098,6 +1106,7 @@ static int read_message(struct xs_handle
/* There should only ever be one response pending! */
if (!list_empty(&h->reply_list)) {
mutex_unlock(&h->reply_mutex);
+ saved_errno = EEXIST;
goto error_freebody;
}
@@ -1110,9 +1119,9 @@ static int read_message(struct xs_handle
ret = 0;
error_freebody:
- cleanup_pop(ret == -1);
+ cleanup_pop_heap(ret == -1, body);
error_freemsg:
- cleanup_pop(ret == -1);
+ cleanup_pop_heap(ret == -1, msg);
error:
errno = saved_errno;
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] Fix libxenstore memory leak when USE_PTHREAD is not defined, V2
2012-09-14 14:58 [PATCH] Fix libxenstore memory leak when USE_PTHREAD is not defined, V2 Andres Lagar-Cavilla
@ 2012-09-14 15:10 ` Ian Campbell
2012-09-17 10:18 ` Ian Campbell
0 siblings, 1 reply; 3+ messages in thread
From: Ian Campbell @ 2012-09-14 15:10 UTC (permalink / raw)
To: Andres Lagar-Cavilla
Cc: andres@gridcentric.ca, Ian Jackson, xen-devel@lists.xen.org
On Fri, 2012-09-14 at 15:58 +0100, Andres Lagar-Cavilla wrote:
> tools/xenstore/xs.c | 17 +++++++++++++----
> 1 files changed, 13 insertions(+), 4 deletions(-)
>
>
> Redefine usage of pthread_cleanup_push and _pop, to explicitly call free for
> heap objects in error paths.
>
> By the way, set a suitable errno value for an error path that had none.
>
> This is V2 after comments from Ian Campbell.
>
> Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>
Looks good to me
Acked-by: Ian Campbell <ian.campbell@citrix.com>
>
> diff -r 588d0dc298a4 -r a83fd69dcb27 tools/xenstore/xs.c
> --- a/tools/xenstore/xs.c
> +++ b/tools/xenstore/xs.c
> @@ -110,6 +110,11 @@ struct xs_handle {
>
> #define read_thread_exists(h) (h->read_thr_exists)
>
> +/* Because pthread_cleanup_p* are not available when USE_PTHREAD is
> + * disabled, use these macros which convert appropriately. */
> +#define cleanup_push_heap(p) cleanup_push(free, p)
> +#define cleanup_pop_heap(run, p) cleanup_pop((run))
> +
> static void *read_thread(void *arg);
>
> #else /* !defined(USE_PTHREAD) */
> @@ -130,6 +135,9 @@ struct xs_handle {
> #define cleanup_pop(run) ((void)0)
> #define read_thread_exists(h) (0)
>
> +#define cleanup_push_heap(p) ((void)0)
> +#define cleanup_pop_heap(run, p) do { if ((run)) free(p); } while(0)
> +
> #endif
>
> static int read_message(struct xs_handle *h, int nonblocking);
> @@ -1059,7 +1067,7 @@ static int read_message(struct xs_handle
> msg = malloc(sizeof(*msg));
> if (msg == NULL)
> goto error;
> - cleanup_push(free, msg);
> + cleanup_push_heap(msg);
> if (!read_all(h->fd, &msg->hdr, sizeof(msg->hdr), nonblocking)) { /* Cancellation point */
> saved_errno = errno;
> goto error_freemsg;
> @@ -1069,7 +1077,7 @@ static int read_message(struct xs_handle
> body = msg->body = malloc(msg->hdr.len + 1);
> if (body == NULL)
> goto error_freemsg;
> - cleanup_push(free, body);
> + cleanup_push_heap(body);
> if (!read_all(h->fd, body, msg->hdr.len, 0)) { /* Cancellation point */
> saved_errno = errno;
> goto error_freebody;
> @@ -1098,6 +1106,7 @@ static int read_message(struct xs_handle
> /* There should only ever be one response pending! */
> if (!list_empty(&h->reply_list)) {
> mutex_unlock(&h->reply_mutex);
> + saved_errno = EEXIST;
> goto error_freebody;
> }
>
> @@ -1110,9 +1119,9 @@ static int read_message(struct xs_handle
> ret = 0;
>
> error_freebody:
> - cleanup_pop(ret == -1);
> + cleanup_pop_heap(ret == -1, body);
> error_freemsg:
> - cleanup_pop(ret == -1);
> + cleanup_pop_heap(ret == -1, msg);
> error:
> errno = saved_errno;
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] Fix libxenstore memory leak when USE_PTHREAD is not defined, V2
2012-09-14 15:10 ` Ian Campbell
@ 2012-09-17 10:18 ` Ian Campbell
0 siblings, 0 replies; 3+ messages in thread
From: Ian Campbell @ 2012-09-17 10:18 UTC (permalink / raw)
To: Andres Lagar-Cavilla
Cc: andres@gridcentric.ca, Ian Jackson, xen-devel@lists.xen.org
On Fri, 2012-09-14 at 16:10 +0100, Ian Campbell wrote:
> On Fri, 2012-09-14 at 15:58 +0100, Andres Lagar-Cavilla wrote:
> > tools/xenstore/xs.c | 17 +++++++++++++----
> > 1 files changed, 13 insertions(+), 4 deletions(-)
> >
> >
> > Redefine usage of pthread_cleanup_push and _pop, to explicitly call free for
> > heap objects in error paths.
> >
> > By the way, set a suitable errno value for an error path that had none.
> >
> > This is V2 after comments from Ian Campbell.
> >
> > Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>
>
> Looks good to me
>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
and applied, thanks.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-09-17 10:18 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-14 14:58 [PATCH] Fix libxenstore memory leak when USE_PTHREAD is not defined, V2 Andres Lagar-Cavilla
2012-09-14 15:10 ` Ian Campbell
2012-09-17 10:18 ` Ian Campbell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).