[Qemu-devel] [PATCH] xtensa: use g_malloc/g_new/g_free

Paolo Bonzini posted 1 patch 6 years, 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170508073529.23449-1-pbonzini@redhat.com
Test checkpatch passed
Test docker passed
Test s390x passed
hw/xtensa/sim.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
[Qemu-devel] [PATCH] xtensa: use g_malloc/g_new/g_free
Posted by Paolo Bonzini 6 years, 10 months ago
Replace malloc/free with g_malloc/g_free to get a program exit on
out of memory.

Replace g_malloc with g_new when allocating the MemoryRegion to
get more type safety.

Reported by Coverity.

Cc: Max Filippov <jcmvbkbc@gmail.com>
Cc: qemu-trivial@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/xtensa/sim.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/xtensa/sim.c b/hw/xtensa/sim.c
index d2d1d3a6fd..97307728f7 100644
--- a/hw/xtensa/sim.c
+++ b/hw/xtensa/sim.c
@@ -41,13 +41,13 @@ static void xtensa_create_memory_regions(const XtensaMemory *memory,
                                          const char *name)
 {
     unsigned i;
-    char *num_name = malloc(strlen(name) + sizeof(i) * 3 + 1);
+    char *num_name = g_malloc(strlen(name) + sizeof(i) * 3 + 1);
 
     for (i = 0; i < memory->num; ++i) {
         MemoryRegion *m;
 
         sprintf(num_name, "%s%u", name, i);
-        m = g_malloc(sizeof(*m));
+        m = g_new(MemoryRegion, 1);
         memory_region_init_ram(m, NULL, num_name,
                                memory->location[i].size,
                                &error_fatal);
@@ -55,7 +55,7 @@ static void xtensa_create_memory_regions(const XtensaMemory *memory,
         memory_region_add_subregion(get_system_memory(),
                                     memory->location[i].addr, m);
     }
-    free(num_name);
+    g_free(num_name);
 }
 
 static uint64_t translate_phys_addr(void *opaque, uint64_t addr)
-- 
2.12.2


Re: [Qemu-devel] [PATCH] xtensa: use g_malloc/g_new/g_free
Posted by Philippe Mathieu-Daudé 6 years, 10 months ago
On 05/08/2017 04:35 AM, Paolo Bonzini wrote:
> Replace malloc/free with g_malloc/g_free to get a program exit on
> out of memory.
>
> Replace g_malloc with g_new when allocating the MemoryRegion to
> get more type safety.
>
> Reported by Coverity.
>
> Cc: Max Filippov <jcmvbkbc@gmail.com>
> Cc: qemu-trivial@nongnu.org
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/xtensa/sim.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/xtensa/sim.c b/hw/xtensa/sim.c
> index d2d1d3a6fd..97307728f7 100644
> --- a/hw/xtensa/sim.c
> +++ b/hw/xtensa/sim.c
> @@ -41,13 +41,13 @@ static void xtensa_create_memory_regions(const XtensaMemory *memory,
>                                           const char *name)
>  {
>      unsigned i;
> -    char *num_name = malloc(strlen(name) + sizeof(i) * 3 + 1);
> +    char *num_name = g_malloc(strlen(name) + sizeof(i) * 3 + 1);
>
>      for (i = 0; i < memory->num; ++i) {
>          MemoryRegion *m;
>
>          sprintf(num_name, "%s%u", name, i);
> -        m = g_malloc(sizeof(*m));
> +        m = g_new(MemoryRegion, 1);
>          memory_region_init_ram(m, NULL, num_name,
>                                 memory->location[i].size,
>                                 &error_fatal);
> @@ -55,7 +55,7 @@ static void xtensa_create_memory_regions(const XtensaMemory *memory,
>          memory_region_add_subregion(get_system_memory(),
>                                      memory->location[i].addr, m);
>      }
> -    free(num_name);
> +    g_free(num_name);
>  }
>
>  static uint64_t translate_phys_addr(void *opaque, uint64_t addr)
>

Re: [Qemu-devel] [PATCH] xtensa: use g_malloc/g_new/g_free
Posted by Alex Bennée 6 years, 10 months ago
Paolo Bonzini <pbonzini@redhat.com> writes:

> Replace malloc/free with g_malloc/g_free to get a program exit on
> out of memory.
>
> Replace g_malloc with g_new when allocating the MemoryRegion to
> get more type safety.
>
> Reported by Coverity.
>
> Cc: Max Filippov <jcmvbkbc@gmail.com>
> Cc: qemu-trivial@nongnu.org
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/xtensa/sim.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/xtensa/sim.c b/hw/xtensa/sim.c
> index d2d1d3a6fd..97307728f7 100644
> --- a/hw/xtensa/sim.c
> +++ b/hw/xtensa/sim.c
> @@ -41,13 +41,13 @@ static void xtensa_create_memory_regions(const XtensaMemory *memory,
>                                           const char *name)
>  {
>      unsigned i;
> -    char *num_name = malloc(strlen(name) + sizeof(i) * 3 + 1);
> +    char *num_name = g_malloc(strlen(name) + sizeof(i) * 3 + 1);

It seems a little sketchy using the size of the storage type as a proxy
for the number of bytes used by the format specifier in a format string.
In this sort of situation I'd just use Glib and stop worrying about it:

  GString num_name = g_string_new(NULL);
  ...

      g_string_printf(num_name, "%s%u", name, i);
      m = g_new(MemoryRegion, 1);
      memory_region_init_ram(m, NULL, num_name->str,
                             memory->location[i].size,
                             &error_fatal);

      ...

  g_string_free(name, TRUE);

>
>      for (i = 0; i < memory->num; ++i) {
>          MemoryRegion *m;
>
>          sprintf(num_name, "%s%u", name, i);
> -        m = g_malloc(sizeof(*m));
> +        m = g_new(MemoryRegion, 1);
>          memory_region_init_ram(m, NULL, num_name,
>                                 memory->location[i].size,
>                                 &error_fatal);
> @@ -55,7 +55,7 @@ static void xtensa_create_memory_regions(const XtensaMemory *memory,
>          memory_region_add_subregion(get_system_memory(),
>                                      memory->location[i].addr, m);
>      }
> -    free(num_name);
> +    g_free(num_name);
>  }
>
>  static uint64_t translate_phys_addr(void *opaque, uint64_t addr)


--
Alex Bennée

Re: [Qemu-devel] [PATCH] xtensa: use g_malloc/g_new/g_free
Posted by Max Filippov 6 years, 10 months ago
On Mon, May 8, 2017 at 6:32 AM, Alex Bennée <alex.bennee@linaro.org> wrote:
>> diff --git a/hw/xtensa/sim.c b/hw/xtensa/sim.c
>> index d2d1d3a6fd..97307728f7 100644
>> --- a/hw/xtensa/sim.c
>> +++ b/hw/xtensa/sim.c
>> @@ -41,13 +41,13 @@ static void xtensa_create_memory_regions(const XtensaMemory *memory,
>>                                           const char *name)
>>  {
>>      unsigned i;
>> -    char *num_name = malloc(strlen(name) + sizeof(i) * 3 + 1);
>> +    char *num_name = g_malloc(strlen(name) + sizeof(i) * 3 + 1);
>
> It seems a little sketchy using the size of the storage type as a proxy
> for the number of bytes used by the format specifier in a format string.
> In this sort of situation I'd just use Glib and stop worrying about it:

Ok, I'll send a patch doing it that way.

-- 
Thanks.
-- Max