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. 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. -- Best regards, Vladimir
