Re: [PATCH 7/7] dump: Consolidate elf note function

[Janosch Frank]

On 3/2/22 11:30, Marc-André Lureau wrote:
> Hi
>
> On Tue, Mar 1, 2022 at 6:22 PM Janosch Frank <frankja@linux.ibm.com> wrote:
> > Just like with the other write functions let's move the 32/64 bit elf
> > handling to a function to improve readability.
> >
> > Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> > ---
> >   dump/dump.c | 24 +++++++++++++++++-------
> >   1 file changed, 17 insertions(+), 7 deletions(-)
> >
> > diff --git a/dump/dump.c b/dump/dump.c
> > index 78654b9c27..9ba0392e00 100644
> > --- a/dump/dump.c
> > +++ b/dump/dump.c
> > @@ -507,6 +507,21 @@ static void write_elf_loads(DumpState *s, Error **errp)
> >       }
> >   }
> >
> > +static void write_elf_notes(DumpState *s, Error **errp)
> > +{
> > +    Error *local_err = NULL;
> > +
> > +    if (dump_is_64bit(s)) {
> > +        write_elf64_notes(fd_write_vmcore, s, &local_err);
> > +    } else {
> > +        write_elf32_notes(fd_write_vmcore, s, &local_err);
> > +    }
> > +    if (local_err) {
> > +        error_propagate(errp, local_err);
> > +        return;
> > +    }
>
> Please use "modern"-style ERRP_GUARD(), and indicate failure with a
> bool (see include/qapi/error.h)

Didn't know that's a thing, I'll have a look

> (perhaps this should be preliminary to this series)

So you want me to change all the local_error + error_propagate()s in 
this file?

> > +}
> > +
> >   /* write elf header, PT_NOTE and elf note to vmcore. */
> >   static void dump_begin(DumpState *s, Error **errp)
> >   {
> > @@ -570,13 +585,8 @@ static void dump_begin(DumpState *s, Error **errp)
> >           }
> >       }
> >
> > -    if (dump_is_64bit(s)) {
> > -        /* write notes to vmcore */
> > -        write_elf64_notes(fd_write_vmcore, s, &local_err);
> > -    } else {
> > -        /* write notes to vmcore */
> > -        write_elf32_notes(fd_write_vmcore, s, &local_err);
> > -    }
> > +    /* write notes to vmcore */
> > +    write_elf_notes(s, &local_err);
> >       if (local_err) {
> >           error_propagate(errp, local_err);
> >           return;
> > --
> > 2.32.0