[Qemu-devel] [PATCH v2 1/3] tests/migration: mem leak fix

Mao Zhongyi posted 3 patches 6 years, 5 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v2 1/3] tests/migration: mem leak fix
Posted by Mao Zhongyi 6 years, 5 months ago
‘data’ has the possibility of memory leaks, so use the
glic macros g_autofree recommended by CODING_STYLE.rst
to automatically release the memory that returned from
g_malloc().

Cc: armbru@redhat.com
Cc: laurent@vivier.eu
Cc: tony.nguyen@bt.com
Cc: alex.bennee@linaro.org

Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
---
 tests/migration/stress.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/tests/migration/stress.c b/tests/migration/stress.c
index d9aa4afe92..6cbb2d49d3 100644
--- a/tests/migration/stress.c
+++ b/tests/migration/stress.c
@@ -170,10 +170,10 @@ static unsigned long long now(void)
 static int stressone(unsigned long long ramsizeMB)
 {
     size_t pagesPerMB = 1024 * 1024 / PAGE_SIZE;
-    char *ram = malloc(ramsizeMB * 1024 * 1024);
+    g_autofree char *ram = malloc(ramsizeMB * 1024 * 1024);
     char *ramptr;
     size_t i, j, k;
-    char *data = malloc(PAGE_SIZE);
+    g_autofree char *data = malloc(PAGE_SIZE);
     char *dataptr;
     size_t nMB = 0;
     unsigned long long before, after;
@@ -186,7 +186,6 @@ static int stressone(unsigned long long ramsizeMB)
     if (!data) {
         fprintf(stderr, "%s (%d): ERROR: cannot allocate %d bytes of RAM: %s\n",
                 argv0, gettid(), PAGE_SIZE, strerror(errno));
-        free(ram);
         return -1;
     }
 
@@ -198,8 +197,6 @@ static int stressone(unsigned long long ramsizeMB)
     memset(ram, 0xfe, ramsizeMB * 1024 * 1024);
 
     if (random_bytes(data, PAGE_SIZE) < 0) {
-        free(ram);
-        free(data);
         return -1;
     }
 
@@ -227,9 +224,6 @@ static int stressone(unsigned long long ramsizeMB)
             }
         }
     }
-
-    free(data);
-    free(ram);
 }
 
 
-- 
2.17.1




Re: [Qemu-devel] [PATCH v2 1/3] tests/migration: mem leak fix
Posted by Alex Bennée 6 years, 5 months ago
Mao Zhongyi <maozhongyi@cmss.chinamobile.com> writes:

> ‘data’ has the possibility of memory leaks, so use the
> glic macros g_autofree recommended by CODING_STYLE.rst

nit: glib

> to automatically release the memory that returned from
> g_malloc().
>
> Cc: armbru@redhat.com
> Cc: laurent@vivier.eu
> Cc: tony.nguyen@bt.com
> Cc: alex.bennee@linaro.org
>
> Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  tests/migration/stress.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/tests/migration/stress.c b/tests/migration/stress.c
> index d9aa4afe92..6cbb2d49d3 100644
> --- a/tests/migration/stress.c
> +++ b/tests/migration/stress.c
> @@ -170,10 +170,10 @@ static unsigned long long now(void)
>  static int stressone(unsigned long long ramsizeMB)
>  {
>      size_t pagesPerMB = 1024 * 1024 / PAGE_SIZE;
> -    char *ram = malloc(ramsizeMB * 1024 * 1024);
> +    g_autofree char *ram = malloc(ramsizeMB * 1024 * 1024);
>      char *ramptr;
>      size_t i, j, k;
> -    char *data = malloc(PAGE_SIZE);
> +    g_autofree char *data = malloc(PAGE_SIZE);
>      char *dataptr;
>      size_t nMB = 0;
>      unsigned long long before, after;
> @@ -186,7 +186,6 @@ static int stressone(unsigned long long ramsizeMB)
>      if (!data) {
>          fprintf(stderr, "%s (%d): ERROR: cannot allocate %d bytes of RAM: %s\n",
>                  argv0, gettid(), PAGE_SIZE, strerror(errno));
> -        free(ram);
>          return -1;
>      }
>
> @@ -198,8 +197,6 @@ static int stressone(unsigned long long ramsizeMB)
>      memset(ram, 0xfe, ramsizeMB * 1024 * 1024);
>
>      if (random_bytes(data, PAGE_SIZE) < 0) {
> -        free(ram);
> -        free(data);
>          return -1;
>      }
>
> @@ -227,9 +224,6 @@ static int stressone(unsigned long long ramsizeMB)
>              }
>          }
>      }
> -
> -    free(data);
> -    free(ram);
>  }


--
Alex Bennée

Re: [PATCH v2 1/3] tests/migration: mem leak fix
Posted by Laurent Vivier 6 years, 4 months ago
Le 11/09/2019 à 05:31, Mao Zhongyi a écrit :
> ‘data’ has the possibility of memory leaks, so use the
> glic macros g_autofree recommended by CODING_STYLE.rst
> to automatically release the memory that returned from
> g_malloc().
> 
> Cc: armbru@redhat.com
> Cc: laurent@vivier.eu
> Cc: tony.nguyen@bt.com
> Cc: alex.bennee@linaro.org
> 
> Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
> ---
>  tests/migration/stress.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/tests/migration/stress.c b/tests/migration/stress.c
> index d9aa4afe92..6cbb2d49d3 100644
> --- a/tests/migration/stress.c
> +++ b/tests/migration/stress.c
> @@ -170,10 +170,10 @@ static unsigned long long now(void)
>  static int stressone(unsigned long long ramsizeMB)
>  {
>      size_t pagesPerMB = 1024 * 1024 / PAGE_SIZE;
> -    char *ram = malloc(ramsizeMB * 1024 * 1024);
> +    g_autofree char *ram = malloc(ramsizeMB * 1024 * 1024);
>      char *ramptr;
>      size_t i, j, k;
> -    char *data = malloc(PAGE_SIZE);
> +    g_autofree char *data = malloc(PAGE_SIZE);

So perhaps g_malloc() could be a better choice as it will exit on
allocation failure?

Thanks,
Laurent


Re: [PATCH v2 1/3] tests/migration: mem leak fix
Posted by maozy 6 years, 4 months ago
On 10/1/19 11:31 PM, Laurent Vivier wrote:
> Le 11/09/2019 à 05:31, Mao Zhongyi a écrit :
>> ‘data’ has the possibility of memory leaks, so use the
>> glic macros g_autofree recommended by CODING_STYLE.rst
>> to automatically release the memory that returned from
>> g_malloc().
>>
>> Cc: armbru@redhat.com
>> Cc: laurent@vivier.eu
>> Cc: tony.nguyen@bt.com
>> Cc: alex.bennee@linaro.org
>>
>> Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
>> ---
>>   tests/migration/stress.c | 10 ++--------
>>   1 file changed, 2 insertions(+), 8 deletions(-)
>>
>> diff --git a/tests/migration/stress.c b/tests/migration/stress.c
>> index d9aa4afe92..6cbb2d49d3 100644
>> --- a/tests/migration/stress.c
>> +++ b/tests/migration/stress.c
>> @@ -170,10 +170,10 @@ static unsigned long long now(void)
>>   static int stressone(unsigned long long ramsizeMB)
>>   {
>>       size_t pagesPerMB = 1024 * 1024 / PAGE_SIZE;
>> -    char *ram = malloc(ramsizeMB * 1024 * 1024);
>> +    g_autofree char *ram = malloc(ramsizeMB * 1024 * 1024);
>>       char *ramptr;
>>       size_t i, j, k;
>> -    char *data = malloc(PAGE_SIZE);
>> +    g_autofree char *data = malloc(PAGE_SIZE);
> So perhaps g_malloc() could be a better choice as it will exit on
> allocation failure?

right,  will fix it in next.

Thanks,
Mao
>
> Thanks,
> Laurent
>
>