Re: [PATCH v2 3/6] xen/livepatch: simplify and unify logic in prepare_payload()

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Sep 25, 2024 at 10:37:56AM +0100, Andrew Cooper wrote:
> On 25/09/2024 9:42 am, Roger Pau Monne wrote:
> > diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
> > index 87b3db03e26d..8e61083f23a7 100644
> > --- a/xen/common/livepatch.c
> > +++ b/xen/common/livepatch.c
> > @@ -470,6 +470,31 @@ static int xen_build_id_dep(const struct payload 
> > *payload)
> >      return 0;
> >  }
> >  
> > +/* Parses build-id sections into the given destination. */
> > +static int parse_buildid(const struct livepatch_elf_sec *sec,
> > +                         struct livepatch_build_id *id)
> > +{
> > +    const Elf_Note *n;
> > +    int rc;
> > +
> > +    /* Presence of the sections is ensured by check_special_sections(). */
> > +    ASSERT(sec);
> > +
> > +    n = sec->load_addr;
> > +
> > +    if ( sec->sec->sh_size <= sizeof(*n) )
> > +        return -EINVAL;
> > +
> > +    rc = xen_build_id_check(n, sec->sec->sh_size, &id->p, &id->len);
> 
> I've just realised what is so confusing.
> 
> This function is not a Xen buildid check, it's an ELF buildid note check.
> 
> I'll do a followup patch after yours goes in renaming it.
> 
> Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Yeah, the naming of xen_build_id_check is confusing, as it's not just
a check, it also populates livepatch_build_id fields.  Thought about
renaming it, but the series was already long enough...

Thanks, Roger.