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.mayd...@linaro.org>
wrote:
> On Tue, 3 Oct 2023 at 18:23, ~h0lyalg0rithm <h0lyalg0ri...@git.sr.ht>
> wrote:
> >
> > From: Suraj Shirvankar <surajshirvan...@gmail.com>
> >
> > Signed-off-by: Suraj Shirvankar <surajshirvan...@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
>
