[Qemu-devel] [PATCH] qemu-io: Fix memory leak

[Qemu-devel] [PATCH] qemu-io: Fix memory leak

[Kevin Wolf]

qemu-io leaks the request buffer whenever the read or write function isn't
executed completely down the "normal" code path.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qemu-io.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/qemu-io.c b/qemu-io.c
index a8d55fe..98ebb65 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -334,7 +334,7 @@ read_f(int argc, char **argv)
 
 	if (cnt < 0) {
 		printf("read failed: %s\n", strerror(-cnt));
-		return 0;
+		goto out;
 	}
 
 	if (Pflag) {
@@ -349,7 +349,7 @@ read_f(int argc, char **argv)
 	}
 
 	if (qflag)
-		return 0;
+		goto out;
 
         if (vflag)
 		dump_buffer(buf, offset, count);
@@ -358,6 +358,7 @@ read_f(int argc, char **argv)
 	t2 = tsub(t2, t1);
 	print_report("read", &t2, offset, count, total, cnt, Cflag);
 
+out:
 	qemu_io_free(buf);
 
 	return 0;
@@ -632,16 +633,17 @@ write_f(int argc, char **argv)
 
 	if (cnt < 0) {
 		printf("write failed: %s\n", strerror(-cnt));
-		return 0;
+		goto out;
 	}
 
 	if (qflag)
-		return 0;
+		goto out;
 
 	/* Finally, report back -- -C gives a parsable format */
 	t2 = tsub(t2, t1);
 	print_report("wrote", &t2, offset, count, total, cnt, Cflag);
 
+out:
 	qemu_io_free(buf);
 
 	return 0;
-- 
1.6.0.6

Re: [Qemu-devel] [PATCH] qemu-io: Fix memory leak

[Christoph Hellwig]

On Wed, Jul 08, 2009 at 02:21:35PM +0200, Kevin Wolf wrote:
> qemu-io leaks the request buffer whenever the read or write function isn't
> executed completely down the "normal" code path.

Looks good.  The same problems also happens in the vectored and aio
read/write command, I'll cook up a patch to fix those.

[Qemu-devel] [PATCH] qemu-io: Fix memory leak

[Kevin Wolf]

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qemu-io.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/qemu-io.c b/qemu-io.c
index cac72e9..c84b361 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -129,7 +129,8 @@ create_iovec(QEMUIOVector *qiov, char **argv, int nr_iov, int pattern)
 {
 	size_t *sizes = calloc(nr_iov, sizeof(size_t));
 	size_t count = 0;
-	void *buf, *p;
+	void *buf = NULL;
+	void *p;
 	int i;
 
 	for (i = 0; i < nr_iov; i++) {
@@ -139,19 +140,19 @@ create_iovec(QEMUIOVector *qiov, char **argv, int nr_iov, int pattern)
 		len = cvtnum(arg);
 		if (len < 0) {
 			printf("non-numeric length argument -- %s\n", arg);
-			return NULL;
+			goto fail;
 		}
 
 		/* should be SIZE_T_MAX, but that doesn't exist */
 		if (len > UINT_MAX) {
 			printf("too large length argument -- %s\n", arg);
-			return NULL;
+			goto fail;
 		}
 
 		if (len & 0x1ff) {
 			printf("length argument %lld is not sector aligned\n",
 				len);
-			return NULL;
+			goto fail;
 		}
 
 		sizes[i] = len;
@@ -167,6 +168,7 @@ create_iovec(QEMUIOVector *qiov, char **argv, int nr_iov, int pattern)
 		p += sizes[i];
 	}
 
+fail:
 	free(sizes);
 	return buf;
 }
-- 
1.6.2.5

Re: [Qemu-devel] [PATCH] qemu-io: Fix memory leak

[Christoph Hellwig]

Looks good,


Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [Qemu-devel] [PATCH] qemu-io: Fix memory leak

[Amit Shah]

On (Wed) Nov 18 2009 [10:42:59], Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  qemu-io.c |   10 ++++++----
>  1 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/qemu-io.c b/qemu-io.c
> index cac72e9..c84b361 100644
> --- a/qemu-io.c
> +++ b/qemu-io.c
> @@ -129,7 +129,8 @@ create_iovec(QEMUIOVector *qiov, char **argv, int nr_iov, int pattern)
>  {
>  	size_t *sizes = calloc(nr_iov, sizeof(size_t));
>  	size_t count = 0;
> -	void *buf, *p;
> +	void *buf = NULL;
> +	void *p;
>  	int i;

I'd prefer the init to happen after the declarations -- brings in
consistent style, puts declarations in one blob and makes
initialisations explicit.


		Amit

Re: [Qemu-devel] [PATCH] qemu-io: Fix memory leak

[Kevin Wolf]

Am 20.11.2009 09:05, schrieb Amit Shah:
> On (Wed) Nov 18 2009 [10:42:59], Kevin Wolf wrote:
>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>> ---
>>  qemu-io.c |   10 ++++++----
>>  1 files changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/qemu-io.c b/qemu-io.c
>> index cac72e9..c84b361 100644
>> --- a/qemu-io.c
>> +++ b/qemu-io.c
>> @@ -129,7 +129,8 @@ create_iovec(QEMUIOVector *qiov, char **argv, int nr_iov, int pattern)
>>  {
>>  	size_t *sizes = calloc(nr_iov, sizeof(size_t));
>>  	size_t count = 0;
>> -	void *buf, *p;
>> +	void *buf = NULL;
>> +	void *p;
>>  	int i;
> 
> I'd prefer the init to happen after the declarations -- brings in
> consistent style, puts declarations in one blob and makes
> initialisations explicit.

In the context of this function it would be inconsistent, and I'd be
surprised if the rest of the qemu code was consistent with your
expectations. After all, it's a matter of taste, and in such questions I
tend to stick with the style of the surrounding code.

Kevin