Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> writes:
> 01.04.2020 12:02, Markus Armbruster wrote:
>> QEMU's Error was patterned after GLib's GError. Differences include:
>> * &error_fatal, &error_abort for convenience
>> * Error can optionally store hints
>> * Pointlessly different names: error_prepend() vs. g_error_prefix()
>> and
>> so forth *shrug*
>> * Propagating errors
>> Thanks to Vladimir, we'll soon have "auto propagation", which is
>> less
>> verbose and less error-prone.
>> * Accumulating errors
>> error_propagate() permits it, g_propagate_error() does not[*].
>> I believe this feature is used rarely. Perhaps we'd be better
>> off
>> without it. The problem is identifying its uses. If I remember
>> correctly, Vladimir struggled with that for his "auto propagation"
>> work.
>> Perhaps "auto propagation" will reduce the number of manual
>> error_propagate() to the point where we can identify accumulations.
>> Removing the feature would become feasible then.
>> * Distinguishing different errors
>> Where Error has ErrorClass, GError has Gquark domain, gint code.
>> Use
>> of ErrorClass other than ERROR_CLASS_GENERIC_ERROR is strongly
>> discouraged. When we need callers to distinguish errors, we return
>> suitable error codes separately.
>> * Return value conventions
>> Common: non-void functions return a distinct error value on
>> failure
>> when such a value can be defined. Patterns:
>> - Functions returning non-null pointers on success return null
>> pointer
>> on failure.
>> - Functions returning non-negative integers on success return a
>> negative error code on failure.
>> Different: GLib discourages void functions, because these lead to
>> awkward error checking code. We have tons of them, and tons of
>> awkward error checking code:
>> Error *err = NULL;
>> frobnicate(arg, &err);
>> if (err) {
>> ... recover ...
>> error_propagate(errp, err);
>> }
>> instead of
>> if (!frobnicate(arg, errp))
>> ... recover ...
>> }
>> Can also lead to pointless creation of Error objects.
>> I consider this a design mistake. Can we still fix it? We have
>> more
>> than 2000 void functions taking an Error ** parameter...
>> Transforming code that receives and checks for errors with
>> Coccinelle
>> shouldn't be hard. Transforming code that returns errors seems more
>> difficult. We need to transform explicit and implicit return to
>> either return true or return false, depending on what we did to the
>> @errp parameter on the way to the return. Hmm.
>>
>> [*] According to documentation; the code merely calls g_warning() then,
>> in typical GLib fashion.
>>
>
>
> Side question:
>
> Can we somehow implement a possibility to reliably identify file and line
> number
> where error is set by error message?
>
> It's where debug of error-bugs always starts: try to imagine which parts of
> the error
> message are "%s", and how to grep for it in the code, keeping in mind also,
> that error massage may be split into several lines..
>
> Put file:line into each error? Seems too noisy for users.. A lot of errors
> are not
> bugs: use do something wrong and see the error, and understands what he is
> doing
> wrong.. It's not usual practice to print file:line into each message
> for user.
I tend to use __func__ for these things as the result is usually easily
grep-able.
>
>
> But what if we do some kind of mapping file:line <-> error code, so user will
> see
> something like:
>
>
> Error 12345: Device drive-scsi0-0-0-0 is not found
>
> ....
>
> Hmm, maybe, just add one more argument to error_setg:
>
> error_setg(errp, 12345, "Device %s is not found", device_name);
>
> - it's enough grep-able.
--
Alex Bennée
