Re: [PATCH 1/5] hw/core/loader: Make load_elf_hdr() return bool, simplify caller

[Philippe Mathieu-Daudé <philmd@xxxxxxxxxx>]

On 19/11/25 20:12, Markus Armbruster wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@xxxxxxxxxxxxxx> writes:
>
> > On 19.11.25 16:08, Markus Armbruster wrote:
> > > Signed-off-by: Markus Armbruster <armbru@xxxxxxxxxx>
> > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@xxxxxxxxxxxxxx>
> >
> > > ---
> > >    include/hw/loader.h |  4 +++-
> > >    hw/arm/boot.c       |  6 +-----
> > >    hw/core/loader.c    |  8 ++++++--
> > >    hw/riscv/spike.c    | 10 +---------
> > >    4 files changed, 11 insertions(+), 17 deletions(-)
> > > diff --git a/include/hw/loader.h b/include/hw/loader.h
> > > index d035e72748..6f91703503 100644
> > > --- a/include/hw/loader.h
> > > +++ b/include/hw/loader.h
> > > @@ -188,8 +188,10 @@ ssize_t load_elf(const char *filename,
> > >     *
> > >     * Inspect an ELF file's header. Read its full header contents into a
> > >     * buffer and/or determine if the ELF is 64bit.
> > > + *
> > > + * Returns true on success, false on failure.
> > I don't really care, but IMO, it's obvious contract for bool+errp functions, 
> > not worth a comment.
> Nearby function comments all have a "Returns" sentence.  I try to blend
> in :)
New developers might just have to look at a particular API such this
loader one, without knowing the global errp contract. With that in
mind, I'll documents @return everywhere.

> > >     */
> > > -void load_elf_hdr(const char *filename, void *hdr, bool *is64, Error **errp);
> > > +bool load_elf_hdr(const char *filename, void *hdr, bool *is64, Error **errp);
> > >      ssize_t load_aout(const char *filename, hwaddr addr, int max_sz,
> > >                      bool big_endian, hwaddr target_page_size);