[PATCH qemu 1/1] Switch memory management calls to new coding conventions

[PATCH qemu 1/1] Switch memory management calls to new coding conventions

[~h0lyalg0rithm]

From: Suraj Shirvankar <surajshirvankar@gmail.com>

Signed-off-by: Suraj Shirvankar <surajshirvankar@gmail.com>
---
 contrib/elf2dmp/addrspace.c | 4 ++--
 contrib/elf2dmp/main.c      | 4 ++--
 contrib/elf2dmp/pdb.c       | 4 ++--
 contrib/elf2dmp/qemu_elf.c  | 4 ++--
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/contrib/elf2dmp/addrspace.c b/contrib/elf2dmp/addrspace.c
index 64b5d680ad..3bfbb5093c 100644
--- a/contrib/elf2dmp/addrspace.c
+++ b/contrib/elf2dmp/addrspace.c
@@ -72,7 +72,7 @@ int pa_space_create(struct pa_space *ps, QEMU_Elf *qemu_elf)
         }
     }
 
-    ps->block = malloc(sizeof(*ps->block) * ps->block_nr);
+    ps->block = g_new(struct pa_block, ps->block_nr);
     if (!ps->block) {
         return 1;
     }
@@ -97,7 +97,7 @@ int pa_space_create(struct pa_space *ps, QEMU_Elf *qemu_elf)
 void pa_space_destroy(struct pa_space *ps)
 {
     ps->block_nr = 0;
-    free(ps->block);
+    g_free(ps->block);
 }
 
 void va_space_set_dtb(struct va_space *vs, uint64_t dtb)
diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c
index 5db163bdbe..97baf0c0c1 100644
--- a/contrib/elf2dmp/main.c
+++ b/contrib/elf2dmp/main.c
@@ -120,14 +120,14 @@ static KDDEBUGGER_DATA64 *get_kdbg(uint64_t KernBase, struct pdb_reader *pdb,
         }
     }
 
-    kdbg = malloc(kdbg_hdr.Size);
+    kdbg = g_malloc(kdbg_hdr.Size);
     if (!kdbg) {
         return NULL;
     }
 
     if (va_space_rw(vs, KdDebuggerDataBlock, kdbg, kdbg_hdr.Size, 0)) {
         eprintf("Failed to extract entire KDBG\n");
-        free(kdbg);
+        g_free(kdbg);
         return NULL;
     }
 
diff --git a/contrib/elf2dmp/pdb.c b/contrib/elf2dmp/pdb.c
index 6ca5086f02..625001d1cf 100644
--- a/contrib/elf2dmp/pdb.c
+++ b/contrib/elf2dmp/pdb.c
@@ -116,7 +116,7 @@ static void *pdb_ds_read(const PDB_DS_HEADER *header,
 
     nBlocks = (size + header->block_size - 1) / header->block_size;
 
-    buffer = malloc(nBlocks * header->block_size);
+    buffer = g_malloc(nBlocks * header->block_size);
     if (!buffer) {
         return NULL;
     }
@@ -201,7 +201,7 @@ static int pdb_init_symbols(struct pdb_reader *r)
     return 0;
 
 out_symbols:
-    free(symbols);
+    g_free(symbols);
 
     return err;
 }
diff --git a/contrib/elf2dmp/qemu_elf.c b/contrib/elf2dmp/qemu_elf.c
index de6ad744c6..9aa8715108 100644
--- a/contrib/elf2dmp/qemu_elf.c
+++ b/contrib/elf2dmp/qemu_elf.c
@@ -94,7 +94,7 @@ static int init_states(QEMU_Elf *qe)
 
     printf("%zu CPU states has been found\n", cpu_nr);
 
-    qe->state = malloc(sizeof(*qe->state) * cpu_nr);
+    qe->state = g_new(QEMUCPUState*, cpu_nr);
     if (!qe->state) {
         return 1;
     }
@@ -115,7 +115,7 @@ static int init_states(QEMU_Elf *qe)
 
 static void exit_states(QEMU_Elf *qe)
 {
-    free(qe->state);
+    g_free(qe->state);
 }
 
 static bool check_ehdr(QEMU_Elf *qe)
-- 
2.38.5

[PATCH qemu 1/1] Switch memory management calls to new coding conventions

[~h0lyalg0rithm]

From: Suraj Shirvankar <surajshirvankar@gmail.com>

Signed-off-by: Suraj Shirvankar <surajshirvankar@gmail.com>
---
 contrib/elf2dmp/addrspace.c | 4 ++--
 contrib/elf2dmp/main.c      | 4 ++--
 contrib/elf2dmp/pdb.c       | 4 ++--
 contrib/elf2dmp/qemu_elf.c  | 4 ++--
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/contrib/elf2dmp/addrspace.c b/contrib/elf2dmp/addrspace.c
index 64b5d680ad..3bfbb5093c 100644
--- a/contrib/elf2dmp/addrspace.c
+++ b/contrib/elf2dmp/addrspace.c
@@ -72,7 +72,7 @@ int pa_space_create(struct pa_space *ps, QEMU_Elf *qemu_elf)
         }
     }
 
-    ps->block = malloc(sizeof(*ps->block) * ps->block_nr);
+    ps->block = g_new(struct pa_block, ps->block_nr);
     if (!ps->block) {
         return 1;
     }
@@ -97,7 +97,7 @@ int pa_space_create(struct pa_space *ps, QEMU_Elf *qemu_elf)
 void pa_space_destroy(struct pa_space *ps)
 {
     ps->block_nr = 0;
-    free(ps->block);
+    g_free(ps->block);
 }
 
 void va_space_set_dtb(struct va_space *vs, uint64_t dtb)
diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c
index 5db163bdbe..97baf0c0c1 100644
--- a/contrib/elf2dmp/main.c
+++ b/contrib/elf2dmp/main.c
@@ -120,14 +120,14 @@ static KDDEBUGGER_DATA64 *get_kdbg(uint64_t KernBase, struct pdb_reader *pdb,
         }
     }
 
-    kdbg = malloc(kdbg_hdr.Size);
+    kdbg = g_malloc(kdbg_hdr.Size);
     if (!kdbg) {
         return NULL;
     }
 
     if (va_space_rw(vs, KdDebuggerDataBlock, kdbg, kdbg_hdr.Size, 0)) {
         eprintf("Failed to extract entire KDBG\n");
-        free(kdbg);
+        g_free(kdbg);
         return NULL;
     }
 
diff --git a/contrib/elf2dmp/pdb.c b/contrib/elf2dmp/pdb.c
index 6ca5086f02..625001d1cf 100644
--- a/contrib/elf2dmp/pdb.c
+++ b/contrib/elf2dmp/pdb.c
@@ -116,7 +116,7 @@ static void *pdb_ds_read(const PDB_DS_HEADER *header,
 
     nBlocks = (size + header->block_size - 1) / header->block_size;
 
-    buffer = malloc(nBlocks * header->block_size);
+    buffer = g_malloc(nBlocks * header->block_size);
     if (!buffer) {
         return NULL;
     }
@@ -201,7 +201,7 @@ static int pdb_init_symbols(struct pdb_reader *r)
     return 0;
 
 out_symbols:
-    free(symbols);
+    g_free(symbols);
 
     return err;
 }
diff --git a/contrib/elf2dmp/qemu_elf.c b/contrib/elf2dmp/qemu_elf.c
index de6ad744c6..9aa8715108 100644
--- a/contrib/elf2dmp/qemu_elf.c
+++ b/contrib/elf2dmp/qemu_elf.c
@@ -94,7 +94,7 @@ static int init_states(QEMU_Elf *qe)
 
     printf("%zu CPU states has been found\n", cpu_nr);
 
-    qe->state = malloc(sizeof(*qe->state) * cpu_nr);
+    qe->state = g_new(QEMUCPUState*, cpu_nr);
     if (!qe->state) {
         return 1;
     }
@@ -115,7 +115,7 @@ static int init_states(QEMU_Elf *qe)
 
 static void exit_states(QEMU_Elf *qe)
 {
-    free(qe->state);
+    g_free(qe->state);
 }
 
 static bool check_ehdr(QEMU_Elf *qe)
-- 
2.38.5

[PATCH qemu 0/1] contrib/elf2dmp: Conversion of conversions of malloc/calloc/free to g_malloc/g_new/g_free

[~h0lyalg0rithm]

Switch malloc and free calls to new coding standard.
g_new is used where structs are defined and g_malloc is used for simpler
structures

Suraj Shirvankar (1):
  Switch memory management calls to new coding conventions

 contrib/elf2dmp/addrspace.c | 4 ++--
 contrib/elf2dmp/main.c      | 4 ++--
 contrib/elf2dmp/pdb.c       | 4 ++--
 contrib/elf2dmp/qemu_elf.c  | 4 ++--
 4 files changed, 8 insertions(+), 8 deletions(-)

-- 
2.38.5

Re: [PATCH qemu 1/1] Switch memory management calls to new coding conventions

[Peter Maydell]

On Tue, 3 Oct 2023 at 18:23, ~h0lyalg0rithm <h0lyalg0rithm@git.sr.ht> wrote:
>
> From: Suraj Shirvankar <surajshirvankar@gmail.com>
>
> Signed-off-by: Suraj Shirvankar <surajshirvankar@gmail.com>

Hi; thanks for this patch. Mostly it looks good, but I
have a couple of review comments; details below.

> ---
>  contrib/elf2dmp/addrspace.c | 4 ++--
>  contrib/elf2dmp/main.c      | 4 ++--
>  contrib/elf2dmp/pdb.c       | 4 ++--
>  contrib/elf2dmp/qemu_elf.c  | 4 ++--
>  4 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/contrib/elf2dmp/addrspace.c b/contrib/elf2dmp/addrspace.c
> index 64b5d680ad..3bfbb5093c 100644
> --- a/contrib/elf2dmp/addrspace.c
> +++ b/contrib/elf2dmp/addrspace.c
> @@ -72,7 +72,7 @@ int pa_space_create(struct pa_space *ps, QEMU_Elf *qemu_elf)
>          }
>      }
>
> -    ps->block = malloc(sizeof(*ps->block) * ps->block_nr);
> +    ps->block = g_new(struct pa_block, ps->block_nr);
>      if (!ps->block) {
>          return 1;
>      }

Unlike malloc(), g_new() can never fail. So the error check
for NULL becomes redundant, and we can remove it. Similarly
in the other cases below (including the g_malloc() call).

> @@ -97,7 +97,7 @@ int pa_space_create(struct pa_space *ps, QEMU_Elf *qemu_elf)
>  void pa_space_destroy(struct pa_space *ps)
>  {
>      ps->block_nr = 0;
> -    free(ps->block);
> +    g_free(ps->block);
>  }
>
>  void va_space_set_dtb(struct va_space *vs, uint64_t dtb)
> diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c
> index 5db163bdbe..97baf0c0c1 100644
> --- a/contrib/elf2dmp/main.c
> +++ b/contrib/elf2dmp/main.c
> @@ -120,14 +120,14 @@ static KDDEBUGGER_DATA64 *get_kdbg(uint64_t KernBase, struct pdb_reader *pdb,
>          }
>      }
>
> -    kdbg = malloc(kdbg_hdr.Size);
> +    kdbg = g_malloc(kdbg_hdr.Size);
>      if (!kdbg) {
>          return NULL;
>      }
>
>      if (va_space_rw(vs, KdDebuggerDataBlock, kdbg, kdbg_hdr.Size, 0)) {
>          eprintf("Failed to extract entire KDBG\n");
> -        free(kdbg);
> +        g_free(kdbg);
>          return NULL;
>      }

This isn't the only place where we free the memory we
allocate here. The other place is the "free(kdbg)" at the
bottom of the main() function. So for consistency we should
change that also to g_free().

> diff --git a/contrib/elf2dmp/pdb.c b/contrib/elf2dmp/pdb.c
> index 6ca5086f02..625001d1cf 100644
> --- a/contrib/elf2dmp/pdb.c
> +++ b/contrib/elf2dmp/pdb.c
> @@ -116,7 +116,7 @@ static void *pdb_ds_read(const PDB_DS_HEADER *header,
>
>      nBlocks = (size + header->block_size - 1) / header->block_size;
>
> -    buffer = malloc(nBlocks * header->block_size);
> +    buffer = g_malloc(nBlocks * header->block_size);
>      if (!buffer) {
>          return NULL;
>      }

Similarly here the buffer we allocated is usually returned
from this function, assigned to some struct field, and then
free()d much later on. So we should also switch all the other
free() calls in this file to g_free().

We should end up with no calls to free() left at all in
the contrib/elf2dmp/ source files, I think.

> @@ -201,7 +201,7 @@ static int pdb_init_symbols(struct pdb_reader *r)
>      return 0;
>
>  out_symbols:
> -    free(symbols);
> +    g_free(symbols);
>
>      return err;
>  }
> diff --git a/contrib/elf2dmp/qemu_elf.c b/contrib/elf2dmp/qemu_elf.c
> index de6ad744c6..9aa8715108 100644
> --- a/contrib/elf2dmp/qemu_elf.c
> +++ b/contrib/elf2dmp/qemu_elf.c
> @@ -94,7 +94,7 @@ static int init_states(QEMU_Elf *qe)
>
>      printf("%zu CPU states has been found\n", cpu_nr);
>
> -    qe->state = malloc(sizeof(*qe->state) * cpu_nr);
> +    qe->state = g_new(QEMUCPUState*, cpu_nr);
>      if (!qe->state) {
>          return 1;
>      }
> @@ -115,7 +115,7 @@ static int init_states(QEMU_Elf *qe)
>
>  static void exit_states(QEMU_Elf *qe)
>  {
> -    free(qe->state);
> +    g_free(qe->state);
>  }
>
>  static bool check_ehdr(QEMU_Elf *qe)
> --
> 2.38.5

thanks
-- PMM

Re: [PATCH qemu 1/1] Switch memory management calls to new coding conventions

[Suraj Shirvankar]

[-- Attachment #1: Type: text/plain, Size: 4509 bytes --]

Thanks for review.My bad I forgot to change the remaining frees.Going to
send another patch

On Mon, Oct 16, 2023 at 6:49 PM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Tue, 3 Oct 2023 at 18:23, ~h0lyalg0rithm <h0lyalg0rithm@git.sr.ht>
> wrote:
> >
> > From: Suraj Shirvankar <surajshirvankar@gmail.com>
> >
> > Signed-off-by: Suraj Shirvankar <surajshirvankar@gmail.com>
>
> Hi; thanks for this patch. Mostly it looks good, but I
> have a couple of review comments; details below.
>
> > ---
> >  contrib/elf2dmp/addrspace.c | 4 ++--
> >  contrib/elf2dmp/main.c      | 4 ++--
> >  contrib/elf2dmp/pdb.c       | 4 ++--
> >  contrib/elf2dmp/qemu_elf.c  | 4 ++--
> >  4 files changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/contrib/elf2dmp/addrspace.c b/contrib/elf2dmp/addrspace.c
> > index 64b5d680ad..3bfbb5093c 100644
> > --- a/contrib/elf2dmp/addrspace.c
> > +++ b/contrib/elf2dmp/addrspace.c
> > @@ -72,7 +72,7 @@ int pa_space_create(struct pa_space *ps, QEMU_Elf
> *qemu_elf)
> >          }
> >      }
> >
> > -    ps->block = malloc(sizeof(*ps->block) * ps->block_nr);
> > +    ps->block = g_new(struct pa_block, ps->block_nr);
> >      if (!ps->block) {
> >          return 1;
> >      }
>
> Unlike malloc(), g_new() can never fail. So the error check
> for NULL becomes redundant, and we can remove it. Similarly
> in the other cases below (including the g_malloc() call).
>
> > @@ -97,7 +97,7 @@ int pa_space_create(struct pa_space *ps, QEMU_Elf
> *qemu_elf)
> >  void pa_space_destroy(struct pa_space *ps)
> >  {
> >      ps->block_nr = 0;
> > -    free(ps->block);
> > +    g_free(ps->block);
> >  }
> >
> >  void va_space_set_dtb(struct va_space *vs, uint64_t dtb)
> > diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c
> > index 5db163bdbe..97baf0c0c1 100644
> > --- a/contrib/elf2dmp/main.c
> > +++ b/contrib/elf2dmp/main.c
> > @@ -120,14 +120,14 @@ static KDDEBUGGER_DATA64 *get_kdbg(uint64_t
> KernBase, struct pdb_reader *pdb,
> >          }
> >      }
> >
> > -    kdbg = malloc(kdbg_hdr.Size);
> > +    kdbg = g_malloc(kdbg_hdr.Size);
> >      if (!kdbg) {
> >          return NULL;
> >      }
> >
> >      if (va_space_rw(vs, KdDebuggerDataBlock, kdbg, kdbg_hdr.Size, 0)) {
> >          eprintf("Failed to extract entire KDBG\n");
> > -        free(kdbg);
> > +        g_free(kdbg);
> >          return NULL;
> >      }
>
> This isn't the only place where we free the memory we
> allocate here. The other place is the "free(kdbg)" at the
> bottom of the main() function. So for consistency we should
> change that also to g_free().
>
> > diff --git a/contrib/elf2dmp/pdb.c b/contrib/elf2dmp/pdb.c
> > index 6ca5086f02..625001d1cf 100644
> > --- a/contrib/elf2dmp/pdb.c
> > +++ b/contrib/elf2dmp/pdb.c
> > @@ -116,7 +116,7 @@ static void *pdb_ds_read(const PDB_DS_HEADER *header,
> >
> >      nBlocks = (size + header->block_size - 1) / header->block_size;
> >
> > -    buffer = malloc(nBlocks * header->block_size);
> > +    buffer = g_malloc(nBlocks * header->block_size);
> >      if (!buffer) {
> >          return NULL;
> >      }
>
> Similarly here the buffer we allocated is usually returned
> from this function, assigned to some struct field, and then
> free()d much later on. So we should also switch all the other
> free() calls in this file to g_free().
>
> We should end up with no calls to free() left at all in
> the contrib/elf2dmp/ source files, I think.
>
> > @@ -201,7 +201,7 @@ static int pdb_init_symbols(struct pdb_reader *r)
> >      return 0;
> >
> >  out_symbols:
> > -    free(symbols);
> > +    g_free(symbols);
> >
> >      return err;
> >  }
> > diff --git a/contrib/elf2dmp/qemu_elf.c b/contrib/elf2dmp/qemu_elf.c
> > index de6ad744c6..9aa8715108 100644
> > --- a/contrib/elf2dmp/qemu_elf.c
> > +++ b/contrib/elf2dmp/qemu_elf.c
> > @@ -94,7 +94,7 @@ static int init_states(QEMU_Elf *qe)
> >
> >      printf("%zu CPU states has been found\n", cpu_nr);
> >
> > -    qe->state = malloc(sizeof(*qe->state) * cpu_nr);
> > +    qe->state = g_new(QEMUCPUState*, cpu_nr);
> >      if (!qe->state) {
> >          return 1;
> >      }
> > @@ -115,7 +115,7 @@ static int init_states(QEMU_Elf *qe)
> >
> >  static void exit_states(QEMU_Elf *qe)
> >  {
> > -    free(qe->state);
> > +    g_free(qe->state);
> >  }
> >
> >  static bool check_ehdr(QEMU_Elf *qe)
> > --
> > 2.38.5
>
> thanks
> -- PMM
>

[-- Attachment #2: Type: text/html, Size: 5784 bytes --]