| Message ID | 20200707160613.848843-3-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Less clumsy error checking | expand |
On 7/7/20 11:05 AM, Markus Armbruster wrote: > Add headlines to the big comment. > > Explain examples for NULL, &error_abort and &error_fatal argument > better. > > Tweak rationale for error_propagate_prepend(). > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > include/qapi/error.h | 51 +++++++++++++++++++++++++++++++------------- > 1 file changed, 36 insertions(+), 15 deletions(-) > Reviewed-by: Eric Blake <eblake@redhat.com>
On Tue, 7 Jul 2020 18:05:30 +0200 Markus Armbruster <armbru@redhat.com> wrote: > Add headlines to the big comment. > > Explain examples for NULL, &error_abort and &error_fatal argument > better. > > Tweak rationale for error_propagate_prepend(). > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- Reviewed-by: Greg Kurz <groug@kaod.org> > include/qapi/error.h | 51 +++++++++++++++++++++++++++++++------------- > 1 file changed, 36 insertions(+), 15 deletions(-) > > diff --git a/include/qapi/error.h b/include/qapi/error.h > index e8960eaad5..6d079c58b7 100644 > --- a/include/qapi/error.h > +++ b/include/qapi/error.h > @@ -15,6 +15,8 @@ > /* > * Error reporting system loosely patterned after Glib's GError. > * > + * = Creating errors = > + * > * Create an error: > * error_setg(&err, "situation normal, all fouled up"); > * > @@ -27,6 +29,8 @@ > * error_setg(&err, "invalid quark\n" // WRONG! > * "Valid quarks are up, down, strange, charm, top, bottom."); > * > + * = Reporting and destroying errors = > + * > * Report an error to the current monitor if we have one, else stderr: > * error_report_err(err); > * This frees the error object. > @@ -40,6 +44,30 @@ > * error_free(err); > * Note that this loses hints added with error_append_hint(). > * > + * Call a function ignoring errors: > + * foo(arg, NULL); > + * This is more concise than > + * Error *err = NULL; > + * foo(arg, &err); > + * error_free(err); // don't do this > + * > + * Call a function aborting on errors: > + * foo(arg, &error_abort); > + * This is more concise and fails more nicely than > + * Error *err = NULL; > + * foo(arg, &err); > + * assert(!err); // don't do this > + * > + * Call a function treating errors as fatal: > + * foo(arg, &error_fatal); > + * This is more concise than > + * Error *err = NULL; > + * foo(arg, &err); > + * if (err) { // don't do this > + * error_report_err(err); > + * exit(1); > + * } > + * > * Handle an error without reporting it (just for completeness): > * error_free(err); > * > @@ -47,6 +75,11 @@ > * reporting it (primarily useful in testsuites): > * error_free_or_abort(&err); > * > + * = Passing errors around = > + * > + * Errors get passed to the caller through the conventional @errp > + * parameter. > + * > * Pass an existing error to the caller: > * error_propagate(errp, err); > * where Error **errp is a parameter, by convention the last one. > @@ -54,11 +87,10 @@ > * Pass an existing error to the caller with the message modified: > * error_propagate_prepend(errp, err, > * "Could not frobnicate '%s': ", name); > - * > - * Avoid > - * error_propagate(errp, err); > + * This is more concise than > + * error_propagate(errp, err); // don't do this > * error_prepend(errp, "Could not frobnicate '%s': ", name); > - * because this fails to prepend when @errp is &error_fatal. > + * and works even when @errp is &error_fatal. > * > * Create a new error and pass it to the caller: > * error_setg(errp, "situation normal, all fouled up"); > @@ -70,15 +102,6 @@ > * handle the error... > * } > * > - * Call a function ignoring errors: > - * foo(arg, NULL); > - * > - * Call a function aborting on errors: > - * foo(arg, &error_abort); > - * > - * Call a function treating errors as fatal: > - * foo(arg, &error_fatal); > - * > * Receive an error and pass it on to the caller: > * Error *err = NULL; > * foo(arg, &err); > @@ -86,8 +109,6 @@ > * handle the error... > * error_propagate(errp, err); > * } > - * where Error **errp is a parameter, by convention the last one. > - * > * Do *not* "optimize" this to > * foo(arg, errp); > * if (*errp) { // WRONG!
diff --git a/include/qapi/error.h b/include/qapi/error.h index e8960eaad5..6d079c58b7 100644 --- a/include/qapi/error.h +++ b/include/qapi/error.h @@ -15,6 +15,8 @@ /* * Error reporting system loosely patterned after Glib's GError. * + * = Creating errors = + * * Create an error: * error_setg(&err, "situation normal, all fouled up"); * @@ -27,6 +29,8 @@ * error_setg(&err, "invalid quark\n" // WRONG! * "Valid quarks are up, down, strange, charm, top, bottom."); * + * = Reporting and destroying errors = + * * Report an error to the current monitor if we have one, else stderr: * error_report_err(err); * This frees the error object. @@ -40,6 +44,30 @@ * error_free(err); * Note that this loses hints added with error_append_hint(). * + * Call a function ignoring errors: + * foo(arg, NULL); + * This is more concise than + * Error *err = NULL; + * foo(arg, &err); + * error_free(err); // don't do this + * + * Call a function aborting on errors: + * foo(arg, &error_abort); + * This is more concise and fails more nicely than + * Error *err = NULL; + * foo(arg, &err); + * assert(!err); // don't do this + * + * Call a function treating errors as fatal: + * foo(arg, &error_fatal); + * This is more concise than + * Error *err = NULL; + * foo(arg, &err); + * if (err) { // don't do this + * error_report_err(err); + * exit(1); + * } + * * Handle an error without reporting it (just for completeness): * error_free(err); * @@ -47,6 +75,11 @@ * reporting it (primarily useful in testsuites): * error_free_or_abort(&err); * + * = Passing errors around = + * + * Errors get passed to the caller through the conventional @errp + * parameter. + * * Pass an existing error to the caller: * error_propagate(errp, err); * where Error **errp is a parameter, by convention the last one. @@ -54,11 +87,10 @@ * Pass an existing error to the caller with the message modified: * error_propagate_prepend(errp, err, * "Could not frobnicate '%s': ", name); - * - * Avoid - * error_propagate(errp, err); + * This is more concise than + * error_propagate(errp, err); // don't do this * error_prepend(errp, "Could not frobnicate '%s': ", name); - * because this fails to prepend when @errp is &error_fatal. + * and works even when @errp is &error_fatal. * * Create a new error and pass it to the caller: * error_setg(errp, "situation normal, all fouled up"); @@ -70,15 +102,6 @@ * handle the error... * } * - * Call a function ignoring errors: - * foo(arg, NULL); - * - * Call a function aborting on errors: - * foo(arg, &error_abort); - * - * Call a function treating errors as fatal: - * foo(arg, &error_fatal); - * * Receive an error and pass it on to the caller: * Error *err = NULL; * foo(arg, &err); @@ -86,8 +109,6 @@ * handle the error... * error_propagate(errp, err); * } - * where Error **errp is a parameter, by convention the last one. - * * Do *not* "optimize" this to * foo(arg, errp); * if (*errp) { // WRONG!
Add headlines to the big comment. Explain examples for NULL, &error_abort and &error_fatal argument better. Tweak rationale for error_propagate_prepend(). Signed-off-by: Markus Armbruster <armbru@redhat.com> --- include/qapi/error.h | 51 +++++++++++++++++++++++++++++++------------- 1 file changed, 36 insertions(+), 15 deletions(-)