Re: [PATCH 12/12] error: Kill @error_warn

[Markus Armbruster <armbru@redhat.com>]

Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp> writes:

> On 2025/08/09 17:30, Markus Armbruster wrote:

[...]

>> As I wrote, the defects are all fixable.  However, there has been so
>> little use of &error_warn, and so much of it has been unclean or
>> otherwise undesirable.  It's obviously prone to misuse.  I think we're
>> better off without it.
>>
>> See also the memo "Abuse of warnings for unhandled errors and
>> programming errors" I posted yesterday.
>
> It is insightful. The cover letter can have a reference to it and this patch can have similar description.
>
> However I still have a few counterarguments:
>
> A reason of the &error_warn usage may be caused by the complexity
> to deal with unhandled errors and programming errors. If so, adding mechanisms to ease that may naturally sufficiently reduce the wrong usage added in the future.
>
> The "&error_fatal without exit(1)" may be good enough for unhandled errors. For example, "[PATCH v10 00/27] migration: propagate vTPM errors using Error objects" would not have used &error_warn if there is the "&error_fatal without exit(1)".

I fear it would be prone to misuse.

Consider

    Error *err = NULL;

    if (!frobnicate(a, b, c, &err)) {
        error_report_err(err);
        // cleanup here, if any
        return false;
    }

With a special &error_fatal_without_exit (terrible name, but you get
what I mean), we could shorten this to

    if (!frobnicate(a, b, c, &error_fatal_without_exit)) {
        // cleanup here, if any
        return false;
    }

However, this invites

    frobnicate(a, b, c, &error_fatal_without_exit);

which is almost always wrong.  If it's an error, we normally can't just
continue as if nothing had happened.

We should make the common case as easy to get rigtht as we can.

Making the exceptions even easier invites misuse.

> There are also warn_report*() functions which also have the same problem. A comprehensive solution needs to deal with them all. Removing all of them will do but may not be practical. Another possibility is that to write a documentation telling warning should be avoided for unhandled/programming errors and let all refer to it.
>
> I agree that there is little valid usage of &error_warn and removing it may not cause a problem at all, but it is still nice if there is a reasoning for the removal, ideally addressing these counterarguments as well.

I can look into improving the commit message after my vacation.

Thanks!