* [PATCH qemu 0/1] contrib/elf2dmp: Conversion of conversions of malloc/calloc/free to g_malloc/g_new/g_free @ 2023-10-03 17:23 ~h0lyalg0rithm 2023-10-03 12:45 ` [PATCH qemu 1/1] Switch memory management calls to new coding conventions ~h0lyalg0rithm 0 siblings, 1 reply; 5+ messages in thread From: ~h0lyalg0rithm @ 2023-10-03 17:23 UTC (permalink / raw) To: qemu-devel; +Cc: trivial 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH qemu 1/1] Switch memory management calls to new coding conventions 2023-10-03 17:23 [PATCH qemu 0/1] contrib/elf2dmp: Conversion of conversions of malloc/calloc/free to g_malloc/g_new/g_free ~h0lyalg0rithm @ 2023-10-03 12:45 ` ~h0lyalg0rithm 2023-10-16 16:48 ` Peter Maydell 0 siblings, 1 reply; 5+ messages in thread From: ~h0lyalg0rithm @ 2023-10-03 12:45 UTC (permalink / raw) To: qemu-devel; +Cc: trivial 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH qemu 1/1] Switch memory management calls to new coding conventions 2023-10-03 12:45 ` [PATCH qemu 1/1] Switch memory management calls to new coding conventions ~h0lyalg0rithm @ 2023-10-16 16:48 ` Peter Maydell 2023-10-17 7:26 ` Suraj Shirvankar 0 siblings, 1 reply; 5+ messages in thread From: Peter Maydell @ 2023-10-16 16:48 UTC (permalink / raw) To: ~h0lyalg0rithm; +Cc: qemu-devel, trivial 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH qemu 1/1] Switch memory management calls to new coding conventions 2023-10-16 16:48 ` Peter Maydell @ 2023-10-17 7:26 ` Suraj Shirvankar 0 siblings, 0 replies; 5+ messages in thread From: Suraj Shirvankar @ 2023-10-17 7:26 UTC (permalink / raw) To: Peter Maydell; +Cc: qemu-devel, trivial [-- 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 --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH qemu 0/1] Elf2dmp: Conversion of conversions of malloc/calloc/free to g_malloc/g_new/g_free @ 2023-10-03 13:10 ~h0lyalg0rithm 2023-10-03 12:45 ` [PATCH qemu 1/1] Switch memory management calls to new coding conventions ~h0lyalg0rithm 0 siblings, 1 reply; 5+ messages in thread From: ~h0lyalg0rithm @ 2023-10-03 13:10 UTC (permalink / raw) To: qemu-devel Based on this issue https://gitlab.com/qemu-project/qemu/-/issues/1798 g_new is used where I think it is applicable and a struct is well defined. 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH qemu 1/1] Switch memory management calls to new coding conventions 2023-10-03 13:10 [PATCH qemu 0/1] Elf2dmp: Conversion of conversions of malloc/calloc/free to g_malloc/g_new/g_free ~h0lyalg0rithm @ 2023-10-03 12:45 ` ~h0lyalg0rithm 0 siblings, 0 replies; 5+ messages in thread From: ~h0lyalg0rithm @ 2023-10-03 12:45 UTC (permalink / raw) To: qemu-devel 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-10-17 12:51 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-10-03 17:23 [PATCH qemu 0/1] contrib/elf2dmp: Conversion of conversions of malloc/calloc/free to g_malloc/g_new/g_free ~h0lyalg0rithm 2023-10-03 12:45 ` [PATCH qemu 1/1] Switch memory management calls to new coding conventions ~h0lyalg0rithm 2023-10-16 16:48 ` Peter Maydell 2023-10-17 7:26 ` Suraj Shirvankar -- strict thread matches above, loose matches on Subject: below -- 2023-10-03 13:10 [PATCH qemu 0/1] Elf2dmp: Conversion of conversions of malloc/calloc/free to g_malloc/g_new/g_free ~h0lyalg0rithm 2023-10-03 12:45 ` [PATCH qemu 1/1] Switch memory management calls to new coding conventions ~h0lyalg0rithm
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).