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

[Markus Armbruster]

Thomas Huth <thuth@redhat.com> writes:

> On 17/03/2022 08.40, Markus Armbruster wrote:
>> Thomas Huth <thuth@redhat.com> writes:
>> 
>>> 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>

[...]

>>>> 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!
>>
>> I'm not sure the issue tracker is a good fit here.  An issue tracker
>> works best when you use it to track units of work that can be completed
>> in one go.  An issue then tracks progress of its unit of work.
>
> That's why I wrote "*some* BiteSizeTasks", not "one BitSizeTask"
> ... of course you've got to break it down for unexperienced new
> developers first, e.g. by subsystem. I did that for example for the
> indentation clean up tasks:
>
>  https://gitlab.com/qemu-project/qemu/-/issues/376
>  https://gitlab.com/qemu-project/qemu/-/issues/371
>  https://gitlab.com/qemu-project/qemu/-/issues/372

Okay, I get you now.

When I flipped the guidance, I also updated a substantial amount of code
to honor the rule:

    Subject: [PATCH v4 00/45] Less clumsy error checking
    Date: Tue,  7 Jul 2020 18:05:28 +0200

    [...]

    This series adopts the GError rule (in writing), and updates a
    substantial amount of code to honor the rule.  Cuts the number of
    error_propagate() calls nearly by half.  The diffstat speaks for
    itself.

    https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg02580.html

The "substantial amount of code" were a few subsystems with many, many
callers.  What remains, as far as I can tell, is many functions with few
callers each.

If I run into a bite-sized conversion task I'd like a volunteer to
tackle, I should file an issue for it.

But collecting the everything to be converted, then partitioning it into
sane parts (*many* parts), then creating an issue for each, is not going
to happen.  Nobody got time for that.

>> This isn't a unit, it's more like a class of units.
>> I added an item to https://wiki.qemu.org/ToDo/CodeTransitions for
>> now.
>
> Thanks, but I doubt that this will help much - the description is
> really terse, and for anybody who is not involved in this email thread
> here, I guess it's hard for them to figure out the related parts in
> the include/qapi/error.h on their own ... so if you really want
> somebody else to work on this topic, I think you have to elaborate
> there a little bit (e.g. by giving an example in-place).

"as explained in include/qapi/error.h" points to the full explanation,
which includes examples.