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

[Thomas Huth <thuth@redhat.com>]

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