| Message ID | 1f763d85b66d3767b96afaa59d483005bb3951e5.1441667360.git.crosthwaite.peter@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 09/10/2015 11:33 PM, Peter Crosthwaite wrote: > Add an API to prefix an already set error with a caller-centric > message. > > If multiple errors are set, all are prefixed individually. > Might be nice to rebase your series to add this first, prior to multi-error support. (That is, while prefixing multiple errors requires multi-error support, adding caller-centric prefix might be useful even now without multi-error). > Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com> > --- > > include/qapi/error.h | 6 ++++++ > util/error.c | 26 ++++++++++++++++++++++++++ > 2 files changed, 32 insertions(+) > > diff --git a/include/qapi/error.h b/include/qapi/error.h > index f44c451..b25c72f 100644 > --- a/include/qapi/error.h > +++ b/include/qapi/error.h > @@ -78,6 +78,12 @@ ErrorClass error_get_class(const Error *err); > Error *error_copy(const Error *err); > > /** > + * Prefix an error message with a formatted string. > + */ > + > +void error_prefix(Error *err, const char *fmt, ...); The string is basically only adding details for human consumption. > +++ b/util/error.c > @@ -19,6 +19,7 @@ struct Error > char *msg; > ErrorClass err_class; > struct Error *next; > + bool prefixed; > }; I'm not sure I follow why this field is needed. > > Error *error_abort; > @@ -142,6 +143,26 @@ const char *error_get_pretty(Error *err) > return err->msg; > } > > +void error_prefix(Error *err, const char *fmt, ...) { Brace on its own line. > + char *msg; > + char *fmt_full; > + va_list ap; > + > + if (!err || err->prefixed) { > + return; > + } > + err->prefixed = true; > + > + msg = err->msg; > + fmt_full = g_strdup_printf("%s%%s", fmt); > + > + va_start(ap, fmt); > + err->msg = g_strdup_printf(fmt_full, ap, msg); I don't think that is portable. Passing va_list as an argument to plain printf just isn't going to do what you think. (There has been a request to add recursive printf via %r, http://austingroupbugs.net/view.php?id=800, but glibc does not support the extension yet) It's not to say that you can't chain, just that you can't chain like this. I don't know if using GString internally to error would make the matter any easier (but one of the patches that will probably land before yours, and thus affect your need to rebase, is mine which adds use of GString for adding a human-readable SUFFIX rather than prefix: https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg02818.html) > + va_end(ap); > + g_free(fmt_full); > + g_free(msg); > +} > + > void error_report_err(Error *err) > { > error_report("%s", error_get_pretty(err)); > @@ -170,6 +191,11 @@ void error_propagate(Error **dst_errp, Error *local_err) > > *dst_errp = local_err; > for (i = local_err; i; i = i->next) { > + /* Propagation implies that the caller is no longer the owner of the > + * error. Therefore reset prefixes, so higher level handlers can > + * prefix again. > + */ > + i->prefixed = false; I'm still not quite sure I buy the semantics of this flag. > dst_errp = &i->next; > } > *dst_errp = old_dst_err; >
diff --git a/include/qapi/error.h b/include/qapi/error.h index f44c451..b25c72f 100644 --- a/include/qapi/error.h +++ b/include/qapi/error.h @@ -78,6 +78,12 @@ ErrorClass error_get_class(const Error *err); Error *error_copy(const Error *err); /** + * Prefix an error message with a formatted string. + */ + +void error_prefix(Error *err, const char *fmt, ...); + +/** * Get a human readable representation of an error object. */ const char *error_get_pretty(Error *err); diff --git a/util/error.c b/util/error.c index 890ce58..e9c23ce 100644 --- a/util/error.c +++ b/util/error.c @@ -19,6 +19,7 @@ struct Error char *msg; ErrorClass err_class; struct Error *next; + bool prefixed; }; Error *error_abort; @@ -142,6 +143,26 @@ const char *error_get_pretty(Error *err) return err->msg; } +void error_prefix(Error *err, const char *fmt, ...) { + char *msg; + char *fmt_full; + va_list ap; + + if (!err || err->prefixed) { + return; + } + err->prefixed = true; + + msg = err->msg; + fmt_full = g_strdup_printf("%s%%s", fmt); + + va_start(ap, fmt); + err->msg = g_strdup_printf(fmt_full, ap, msg); + va_end(ap); + g_free(fmt_full); + g_free(msg); +} + void error_report_err(Error *err) { error_report("%s", error_get_pretty(err)); @@ -170,6 +191,11 @@ void error_propagate(Error **dst_errp, Error *local_err) *dst_errp = local_err; for (i = local_err; i; i = i->next) { + /* Propagation implies that the caller is no longer the owner of the + * error. Therefore reset prefixes, so higher level handlers can + * prefix again. + */ + i->prefixed = false; dst_errp = &i->next; } *dst_errp = old_dst_err;
Add an API to prefix an already set error with a caller-centric message. If multiple errors are set, all are prefixed individually. Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com> --- include/qapi/error.h | 6 ++++++ util/error.c | 26 ++++++++++++++++++++++++++ 2 files changed, 32 insertions(+)