Re: [PATCH] target/s390x/arch_dump: Simplify memory allocation in s390x_write_elf64_notes()

[Philippe Mathieu-Daudé]

On 15/2/23 08:26, Thomas Huth wrote:
> On 15/02/2023 08.10, Philippe Mathieu-Daudé wrote:
> > On 15/2/23 06:48, Thomas Huth wrote:
> > > We are not on a hot path here, so there is no real need for the logic
> > > here with the split heap and stack space allocation. Simplify it by
> > > always allocating memory from the heap.
> > >
> > > Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > > ---
> > >   Based-on: <20230214141056.680969-1-thuth@redhat.com>
> > >
> > >   target/s390x/arch_dump.c | 20 ++++++--------------
> > >   1 file changed, 6 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/target/s390x/arch_dump.c b/target/s390x/arch_dump.c
> > > index a7c44ba49d..84e17effda 100644
> > > --- a/target/s390x/arch_dump.c
> > > +++ b/target/s390x/arch_dump.c
> > > @@ -227,25 +227,23 @@ static int s390x_write_elf64_notes(const char 
> > > *note_name,
> > >                                          DumpState *s,
> > >                                          const NoteFuncDesc *funcs)
> > >   {
> > > -    Note note, *notep;
> > > +    g_autofree Note *notep = NULL;
> > >       const NoteFuncDesc *nf;
> > > -    int note_size, content_size;
> > > +    int note_size, prev_size = 0, content_size;
> >
> > We can start with:
> >
> >    prev_size = sizeof(Note);
> >
> > If this goes thru your tree, feel free to modify without respining.
>
> But then I'd also need to initialize notep above differently, don't I?
> And if I've got it right, this function sometimes also deals with chunks 
> that are smaller, so I think it's cleaner if we start with zero instead 
> of sizeof(Note).

Oh you are right. We are good then!

> > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> Thanks!
>
>   Thomas