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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 20.09.2024 11:36, Roger Pau Monne wrote:
> The following sections: .note.gnu.build-id, .livepatch.xen_depends and
> .livepatch.depends are mandatory and ensured to be present by
> check_special_sections() before prepare_payload() is called.
> 
> Simplify the logic in prepare_payload() by introducing a generic function to
> parse the sections that contain a buildid.  Note the function assumes the
> buildid related section to always be present.
> 
> No functional change intended.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> ---
>  xen/common/livepatch.c | 106 ++++++++++++++++++-----------------------
>  1 file changed, 46 insertions(+), 60 deletions(-)
> 
> diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
> index d93a556bcda2..cea47ffe4c84 100644
> --- a/xen/common/livepatch.c
> +++ b/xen/common/livepatch.c
> @@ -647,15 +647,37 @@ static inline int livepatch_check_expectations(const 
> struct payload *payload)
>      nhooks = __sec->sec->sh_size / sizeof(*hook);                            
>              \
>  } while (0)
>  
> +static int fetch_buildid(const struct livepatch_elf_sec *sec,
> +                         struct livepatch_build_id *id)
> +{
> +    const Elf_Note *n = sec->load_addr;
> +    int rc;
> +
> +    ASSERT(sec);
> +
> +    if ( sec->sec->sh_size <= sizeof(*n) )
> +        return -EINVAL;

Oh, after my reply to Andrew's reply, now looking at the actual change -
is it perhaps ASSERT(sec->sec) that was meant?

Jan