[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 2/3] module: add Error arguments to module_load_one and mo
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v4 2/3] module: add Error arguments to module_load_one and module_load_qom_one |
Date: |
Thu, 22 Sep 2022 16:36:15 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) |
Claudio Fontana <cfontana@suse.de> writes:
> On 9/22/22 15:20, Markus Armbruster wrote:
>> Claudio Fontana <cfontana@suse.de> writes:
>>
>> [...]
>>
>>> I think it would be better to completely make the return value separate
>>> from the Error,
>>> and really treat Error as an exception and not mix it up with the regular
>>> execution,
>>>
>>> but if it is the general consensus that I am the only one seeing this
>>> conflation problem we can model it this way too.
>>
>> It's a matter of language pragmatics. In Java, you throw an exception
>> on error. In C, you return an error value.
>>
>> Trying to emulate exceptions in C might be even more unadvisable than
>> trying to avoid them in Java. Best to work with the language, not
>> against it.
>>
>> Trouble is the error values we can conveniently return in C can't convey
>> enough information. So we use Error for that. Just like GLib uses
>
> Right, we use Error for that and that's fine, but we should use it _only
> Error_ for that.
>
> Ie map the Exceptions directly to Error.
>
> So:
>
> try {
>
> rv = function_call(...);
>
> use_return_value(rv);
>
> } catch (Exception e) {
>
> /* handle exceptional case */
>
> }
>
> becomes
>
> rv = function_call(..., Error **errp);
>
> if (errp) {
> /* handle exceptional case */
> }
>
> use_return_value(rv);
This is simply not the intended use of Error.
Also, "trying to emulate exceptions in C might be even more unadvisable
than trying to avoid them in Java."
> Instead we mix up the Exception code path and the regular code path, by
> having rv carry a mix of the Exception and regular return value,
> and this creates problems and confusion.
"In C, you return an error value."
> If we put a hard line between the two, I think more clarity emerges.
Maybe, but consistency matters. Clarity doesn't emerge in isolation.
Deviating from prevailing usage tends to confuse.
>> GError.
>>
>> More modern languages do "return error value" much better than C can. C
>> is what it is.
>>
>> We could certainly argue how to do better than we do now in QEMU's C
>> code. However, the Error API is used all over the place, which makes
>> changing it expensive. "Rethinking the whole Error API" (your words)
>> would have to generate benefits worth this expense. Which seems
>> unlikely.
>>
>> [...]
>>
>
> This is all fine, the problem is how we remodel this in C.
>
> This is how I see the next steps to clarify my position:
>
> short term:
>
> - keep the existing API with the existing assumptions, use a separate
> argument to carry the pointer to the actual return value (where the function
> return value as provided by the language is used to return if an exception
> was generated or not, as we assume today).
We don't actually need separate values for "actual return value" and
"success vs. failure" here. We can easily encode them in a single
return value. This is *common* in C, for better or worse.
For instance, read() returns -1 on error, 0 on EOF (which is not an
error), and a positive value on actual read. On error, @errno is set,
which is a bit awkward (we wouldn't design that way today, I hope).
The interface I proposed is similar: return -1 on error, 0 on not found
(which is not an error), and 1 on successful load. On error, an Error
is set via @errp. With a name that makes it obvious that "not found" is
not an error, there's nothing that should surprise someone
passingly familiar with QEMU code.
> - long term (maybe): fix the existing API by detaching completely the return
> value from the exception.
As I wrote, this seems unlikely to be worth its considerable expense.
> Wdyt?
>
> C
- Re: [PATCH v4 2/3] module: add Error arguments to module_load_one and module_load_qom_one, (continued)
- Re: [PATCH v4 2/3] module: add Error arguments to module_load_one and module_load_qom_one, Markus Armbruster, 2022/09/22
- Re: [PATCH v4 2/3] module: add Error arguments to module_load_one and module_load_qom_one, Claudio Fontana, 2022/09/22
- Re: [PATCH v4 2/3] module: add Error arguments to module_load_one and module_load_qom_one, Markus Armbruster, 2022/09/22
- Re: [PATCH v4 2/3] module: add Error arguments to module_load_one and module_load_qom_one, Claudio Fontana, 2022/09/22
- Re: [PATCH v4 2/3] module: add Error arguments to module_load_one and module_load_qom_one, Markus Armbruster, 2022/09/22
- Re: [PATCH v4 2/3] module: add Error arguments to module_load_one and module_load_qom_one, Claudio Fontana, 2022/09/22
- Re: [PATCH v4 2/3] module: add Error arguments to module_load_one and module_load_qom_one,
Markus Armbruster <=
- Re: [PATCH v4 2/3] module: add Error arguments to module_load_one and module_load_qom_one, Claudio Fontana, 2022/09/22
- Re: [PATCH v4 2/3] module: add Error arguments to module_load_one and module_load_qom_one, Markus Armbruster, 2022/09/23
- Re: [PATCH v4 2/3] module: add Error arguments to module_load_one and module_load_qom_one, Claudio Fontana, 2022/09/23
- Re: [PATCH v4 2/3] module: add Error arguments to module_load_one and module_load_qom_one, Philippe Mathieu-Daudé, 2022/09/22
- Re: [PATCH v4 2/3] module: add Error arguments to module_load_one and module_load_qom_one, Claudio Fontana, 2022/09/22
- Re: [PATCH v4 2/3] module: add Error arguments to module_load_one and module_load_qom_one, Daniel P . Berrangé, 2022/09/22
- Re: [PATCH v4 2/3] module: add Error arguments to module_load_one and module_load_qom_one, Claudio Fontana, 2022/09/22
- Re: [PATCH v4 2/3] module: add Error arguments to module_load_one and module_load_qom_one, Kevin Wolf, 2022/09/22
- Re: [PATCH v4 2/3] module: add Error arguments to module_load_one and module_load_qom_one, Claudio Fontana, 2022/09/22
- Re: [PATCH v4 2/3] module: add Error arguments to module_load_one and module_load_qom_one, Markus Armbruster, 2022/09/22