Re: [PATCH 2/3] hw/display: Allow vga_common_init() to return errors

[Thomas Huth]

On 16/03/2022 15.16, Markus Armbruster wrote:
> Thomas Huth <thuth@redhat.com> writes:
>
> > On 16/03/2022 14.32, Philippe Mathieu-Daudé wrote:
> > > On 16/3/22 14:24, Thomas Huth wrote:
> > > > The vga_common_init() function currently cannot report errors to its
> > > > caller. But in the following patch, we'd need this possibility, so
> > > > let's change it to take an "Error **" as parameter for this.
> > > >
> > > > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > > > ---
> > > >    hw/display/ati.c            |  7 ++++++-
> > > >    hw/display/cirrus_vga.c     |  7 ++++++-
> > > >    hw/display/cirrus_vga_isa.c |  7 ++++++-
> > > >    hw/display/qxl.c            |  6 +++++-
> > > >    hw/display/vga-isa.c        |  9 ++++++++-
> > > >    hw/display/vga-mmio.c       |  8 +++++++-
> > > >    hw/display/vga-pci.c        | 15 +++++++++++++--
> > > >    hw/display/vga.c            |  9 +++++++--
> > > >    hw/display/vga_int.h        |  2 +-
> > > >    hw/display/virtio-vga.c     |  7 ++++++-
> > > >    hw/display/vmware_vga.c     |  2 +-
> > > >    11 files changed, 66 insertions(+), 13 deletions(-)
> > >
> > > Please setup scripts/git.orderfile :)
> > >
> > > > diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h
> > > > index 847e784ca6..3e8892df28 100644
> > > > --- a/hw/display/vga_int.h
> > > > +++ b/hw/display/vga_int.h
> > > > @@ -156,7 +156,7 @@ static inline int c6_to_8(int v)
> > > >        return (v << 2) | (b << 1) | b;
> > > >    }
> > > > -void vga_common_init(VGACommonState *s, Object *obj);
> > > > +void vga_common_init(VGACommonState *s, Object *obj, Error **errp);
> > >
> > > Can we also return a boolean value? IIUC Markus recommended to check
> > > a boolean return value rather than Error* handle.
> >
> > Really? A very quick grep shows something different:
> >
> > $ grep -r ^void.*Error include/ | wc -l
> > 94
> > $ grep -r ^bool.*Error include/ | wc -l
> > 46
>
> Historical reasons.  We deviated from GLib here only to find out that
> the deviation leads to awkward code.  I flipped the guidance in commit
> e3fe3988d7 "error: Document Error API usage rules" (2020-07-10).  A lot
> of old code remains.

Hmm, you should add some BiteSizeTasks to our issue tracker then to get this 
fixed, otherwise people like me will copy-n-paste the bad code examples that 
are all over the place!

 Thomas