Message ID | 1378734793-17818-1-git-send-email-mreitz@redhat.com |
---|---|
State | New |
Headers | show |
On 2013-09-09 15:53, Max Reitz wrote: > Add a configure switch which enables an error propagation backtrace. > This results in the error_set function prepending every message by the > source file name, function and line in which it was called, as well as > error_propagate appending this information to the propagated message, > resulting in a backtrace. > > Signed-off-by: Max Reitz <mreitz@redhat.com> Ping – any comments on this? > --- > Since this obviously breaks existing tests (and cannot really be used for > new tests since it includes a line number, which is a rather volatile > output) and is generally not very useful to normal users, the switch is > disabled by default. This functionality is intended for developers > tracking down error paths. > > Since I do not know whether I am the only one considering it useful > in the first place, this is just an RFC for now. > > Furthermore, I am not sure whether a configure switch is really the right > way to implement this. > --- > configure | 12 +++++++++++ > include/qapi/error.h | 40 +++++++++++++++++++++++++++--------- > util/error.c | 57 +++++++++++++++++++++++++++++++++++++++++++--------- > 3 files changed, 90 insertions(+), 19 deletions(-) > > diff --git a/configure b/configure > index e989609..4e43f7b 100755 > --- a/configure > +++ b/configure > @@ -243,6 +243,7 @@ gtk="" > gtkabi="2.0" > tpm="no" > libssh2="" > +error_backtrace="no" > > # parse CC options first > for opt do > @@ -949,6 +950,10 @@ for opt do > ;; > --enable-libssh2) libssh2="yes" > ;; > + --enable-error-bt) error_backtrace="yes" > + ;; > + --disable-error-bt) error_backtrace="no" > + ;; > *) echo "ERROR: unknown option $opt"; show_help="yes" > ;; > esac > @@ -1168,6 +1173,8 @@ echo " --gcov=GCOV use specified gcov [$gcov_tool]" > echo " --enable-tpm enable TPM support" > echo " --disable-libssh2 disable ssh block device support" > echo " --enable-libssh2 enable ssh block device support" > +echo " --disable-error-bt disable backtraces on internal errors" > +echo " --enable-error-bt enable backtraces on internal errors" > echo "" > echo "NOTE: The object files are built at the place where configure is launched" > exit 1 > @@ -3650,6 +3657,7 @@ echo "gcov $gcov_tool" > echo "gcov enabled $gcov" > echo "TPM support $tpm" > echo "libssh2 support $libssh2" > +echo "error backtraces $error_backtrace" > echo "TPM passthrough $tpm_passthrough" > echo "QOM debugging $qom_cast_debug" > > @@ -4044,6 +4052,10 @@ if test "$libssh2" = "yes" ; then > echo "CONFIG_LIBSSH2=y" >> $config_host_mak > fi > > +if test "$error_backtrace" = "yes" ; then > + echo "CONFIG_ERROR_BACKTRACE=y" >> $config_host_mak > +fi > + > if test "$virtio_blk_data_plane" = "yes" ; then > echo 'CONFIG_VIRTIO_BLK_DATA_PLANE=$(CONFIG_VIRTIO)' >> $config_host_mak > fi > diff --git a/include/qapi/error.h b/include/qapi/error.h > index ffd1cea..d3cfe35 100644 > --- a/include/qapi/error.h > +++ b/include/qapi/error.h > @@ -25,16 +25,28 @@ typedef struct Error Error; > /** > * Set an indirect pointer to an error given a ErrorClass value and a > * printf-style human message. This function is not meant to be used outside > - * of QEMU. > + * of QEMU. The message will be prepended by the file/function/line information > + * if CONFIG_ERROR_BACKTRACE is defined. > */ > -void error_set(Error **err, ErrorClass err_class, const char *fmt, ...) GCC_FMT_ATTR(3, 4); > +void error_set_bt(const char *file, const char *func, int line, > + Error **err, ErrorClass err_class, const char *fmt, ...) > + GCC_FMT_ATTR(6, 7); > + > +#define error_set(...) \ > + error_set_bt(__FILE__, __func__, __LINE__, __VA_ARGS__) > > /** > * Set an indirect pointer to an error given a ErrorClass value and a > * printf-style human message, followed by a strerror() string if > - * @os_error is not zero. > + * @os_error is not zero. The message will be prepended by the > + * file/function/line information if CONFIG_ERROR_BACKTRACE is defined. > */ > -void error_set_errno(Error **err, int os_error, ErrorClass err_class, const char *fmt, ...) GCC_FMT_ATTR(4, 5); > +void error_set_errno_bt(const char *file, const char *func, int line, > + Error **err, int os_error, ErrorClass err_class, > + const char *fmt, ...) GCC_FMT_ATTR(7, 8); > + > +#define error_set_errno(...) \ > + error_set_errno_bt(__FILE__, __func__, __LINE__, __VA_ARGS__) > > /** > * Same as error_set(), but sets a generic error > @@ -47,7 +59,11 @@ void error_set_errno(Error **err, int os_error, ErrorClass err_class, const char > /** > * Helper for open() errors > */ > -void error_setg_file_open(Error **errp, int os_errno, const char *filename); > +void error_setg_file_open_bt(const char *file, const char *func, int line, > + Error **errp, int os_errno, const char *filename); > + > +#define error_setg_file_open(...) \ > + error_setg_file_open_bt(__FILE__, __func__, __LINE__, __VA_ARGS__) > > /** > * Returns true if an indirect pointer to an error is pointing to a valid > @@ -71,11 +87,17 @@ Error *error_copy(const Error *err); > const char *error_get_pretty(Error *err); > > /** > - * Propagate an error to an indirect pointer to an error. This function will > - * always transfer ownership of the error reference and handles the case where > - * dst_err is NULL correctly. Errors after the first are discarded. > + * Propagate an error to an indirect pointer to an error, appending the given > + * file/function/line information to the message (as a backtrace) if > + * CONFIG_ERROR_BACKTRACE is defined. This function will always transfer > + * ownership of the error reference and handles the case where dst_err is NULL > + * correctly. Errors after the first are discarded. > */ > -void error_propagate(Error **dst_err, Error *local_err); > +void error_propagate_bt(const char *file, const char *func, int line, > + Error **dest_err, Error *local_err); > + > +#define error_propagate(...) \ > + error_propagate_bt(__FILE__, __func__, __LINE__, __VA_ARGS__) > > /** > * Free an error object. > diff --git a/util/error.c b/util/error.c > index 53b0435..a07f4c7 100644 > --- a/util/error.c > +++ b/util/error.c > @@ -23,9 +23,11 @@ struct Error > ErrorClass err_class; > }; > > -void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...) > +void error_set_bt(const char *file, const char *func, int line, > + Error **errp, ErrorClass err_class, const char *fmt, ...) > { > Error *err; > + char *msg1; > va_list ap; > > if (errp == NULL) { > @@ -36,15 +38,22 @@ void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...) > err = g_malloc0(sizeof(*err)); > > va_start(ap, fmt); > - err->msg = g_strdup_vprintf(fmt, ap); > + msg1 = g_strdup_vprintf(fmt, ap); > +#ifdef CONFIG_ERROR_BACKTRACE > + err->msg = g_strdup_printf("%s:%i (in %s): %s", file, line, func, msg1); > + g_free(msg1); > +#else > + err->msg = msg1; > +#endif > va_end(ap); > err->err_class = err_class; > > *errp = err; > } > > -void error_set_errno(Error **errp, int os_errno, ErrorClass err_class, > - const char *fmt, ...) > +void error_set_errno_bt(const char *file, const char *func, int line, > + Error **errp, int os_errno, ErrorClass err_class, > + const char *fmt, ...) > { > Error *err; > char *msg1; > @@ -59,21 +68,34 @@ void error_set_errno(Error **errp, int os_errno, ErrorClass err_class, > > va_start(ap, fmt); > msg1 = g_strdup_vprintf(fmt, ap); > +#ifdef CONFIG_ERROR_BACKTRACE > + if (os_errno != 0) { > + err->msg = g_strdup_printf("%s:%i (in %s): %s: %s", file, line, func, > + msg1, strerror(os_errno)); > + } else { > + err->msg = g_strdup_printf("%s:%i (in %s): %s", file, line, func, msg1); > + } > + g_free(msg1); > +#else > if (os_errno != 0) { > err->msg = g_strdup_printf("%s: %s", msg1, strerror(os_errno)); > g_free(msg1); > } else { > err->msg = msg1; > } > +#endif > va_end(ap); > err->err_class = err_class; > > *errp = err; > } > > -void error_setg_file_open(Error **errp, int os_errno, const char *filename) > +void error_setg_file_open_bt(const char *file, const char *func, int line, > + Error **errp, int os_errno, const char *filename) > { > - error_setg_errno(errp, os_errno, "Could not open '%s'", filename); > + error_set_errno_bt(file, func, line, errp, os_errno, > + ERROR_CLASS_GENERIC_ERROR, "Could not open '%s'", > + filename); > } > > Error *error_copy(const Error *err) > @@ -110,11 +132,26 @@ void error_free(Error *err) > } > } > > -void error_propagate(Error **dst_err, Error *local_err) > + > +void error_propagate_bt(const char *file, const char *func, int line, > + Error **dst_err, Error *local_err) > { > - if (dst_err && !*dst_err) { > - *dst_err = local_err; > - } else if (local_err) { > + if (local_err) { > +#ifdef CONFIG_ERROR_BACKTRACE > + if (dst_err && !*dst_err) { > + Error *err = g_malloc0(sizeof(*err)); > + err->msg = g_strdup_printf("%s\n from %s:%i (in %s)", > + local_err->msg, file, line, func); > + err->err_class = local_err->err_class; > + *dst_err = err; > + } > error_free(local_err); > +#else > + if (dst_err && !*dst_err) { > + *dst_err = local_err; > + } else { > + error_free(local_err); > + } > +#endif > } > }
diff --git a/configure b/configure index e989609..4e43f7b 100755 --- a/configure +++ b/configure @@ -243,6 +243,7 @@ gtk="" gtkabi="2.0" tpm="no" libssh2="" +error_backtrace="no" # parse CC options first for opt do @@ -949,6 +950,10 @@ for opt do ;; --enable-libssh2) libssh2="yes" ;; + --enable-error-bt) error_backtrace="yes" + ;; + --disable-error-bt) error_backtrace="no" + ;; *) echo "ERROR: unknown option $opt"; show_help="yes" ;; esac @@ -1168,6 +1173,8 @@ echo " --gcov=GCOV use specified gcov [$gcov_tool]" echo " --enable-tpm enable TPM support" echo " --disable-libssh2 disable ssh block device support" echo " --enable-libssh2 enable ssh block device support" +echo " --disable-error-bt disable backtraces on internal errors" +echo " --enable-error-bt enable backtraces on internal errors" echo "" echo "NOTE: The object files are built at the place where configure is launched" exit 1 @@ -3650,6 +3657,7 @@ echo "gcov $gcov_tool" echo "gcov enabled $gcov" echo "TPM support $tpm" echo "libssh2 support $libssh2" +echo "error backtraces $error_backtrace" echo "TPM passthrough $tpm_passthrough" echo "QOM debugging $qom_cast_debug" @@ -4044,6 +4052,10 @@ if test "$libssh2" = "yes" ; then echo "CONFIG_LIBSSH2=y" >> $config_host_mak fi +if test "$error_backtrace" = "yes" ; then + echo "CONFIG_ERROR_BACKTRACE=y" >> $config_host_mak +fi + if test "$virtio_blk_data_plane" = "yes" ; then echo 'CONFIG_VIRTIO_BLK_DATA_PLANE=$(CONFIG_VIRTIO)' >> $config_host_mak fi diff --git a/include/qapi/error.h b/include/qapi/error.h index ffd1cea..d3cfe35 100644 --- a/include/qapi/error.h +++ b/include/qapi/error.h @@ -25,16 +25,28 @@ typedef struct Error Error; /** * Set an indirect pointer to an error given a ErrorClass value and a * printf-style human message. This function is not meant to be used outside - * of QEMU. + * of QEMU. The message will be prepended by the file/function/line information + * if CONFIG_ERROR_BACKTRACE is defined. */ -void error_set(Error **err, ErrorClass err_class, const char *fmt, ...) GCC_FMT_ATTR(3, 4); +void error_set_bt(const char *file, const char *func, int line, + Error **err, ErrorClass err_class, const char *fmt, ...) + GCC_FMT_ATTR(6, 7); + +#define error_set(...) \ + error_set_bt(__FILE__, __func__, __LINE__, __VA_ARGS__) /** * Set an indirect pointer to an error given a ErrorClass value and a * printf-style human message, followed by a strerror() string if - * @os_error is not zero. + * @os_error is not zero. The message will be prepended by the + * file/function/line information if CONFIG_ERROR_BACKTRACE is defined. */ -void error_set_errno(Error **err, int os_error, ErrorClass err_class, const char *fmt, ...) GCC_FMT_ATTR(4, 5); +void error_set_errno_bt(const char *file, const char *func, int line, + Error **err, int os_error, ErrorClass err_class, + const char *fmt, ...) GCC_FMT_ATTR(7, 8); + +#define error_set_errno(...) \ + error_set_errno_bt(__FILE__, __func__, __LINE__, __VA_ARGS__) /** * Same as error_set(), but sets a generic error @@ -47,7 +59,11 @@ void error_set_errno(Error **err, int os_error, ErrorClass err_class, const char /** * Helper for open() errors */ -void error_setg_file_open(Error **errp, int os_errno, const char *filename); +void error_setg_file_open_bt(const char *file, const char *func, int line, + Error **errp, int os_errno, const char *filename); + +#define error_setg_file_open(...) \ + error_setg_file_open_bt(__FILE__, __func__, __LINE__, __VA_ARGS__) /** * Returns true if an indirect pointer to an error is pointing to a valid @@ -71,11 +87,17 @@ Error *error_copy(const Error *err); const char *error_get_pretty(Error *err); /** - * Propagate an error to an indirect pointer to an error. This function will - * always transfer ownership of the error reference and handles the case where - * dst_err is NULL correctly. Errors after the first are discarded. + * Propagate an error to an indirect pointer to an error, appending the given + * file/function/line information to the message (as a backtrace) if + * CONFIG_ERROR_BACKTRACE is defined. This function will always transfer + * ownership of the error reference and handles the case where dst_err is NULL + * correctly. Errors after the first are discarded. */ -void error_propagate(Error **dst_err, Error *local_err); +void error_propagate_bt(const char *file, const char *func, int line, + Error **dest_err, Error *local_err); + +#define error_propagate(...) \ + error_propagate_bt(__FILE__, __func__, __LINE__, __VA_ARGS__) /** * Free an error object. diff --git a/util/error.c b/util/error.c index 53b0435..a07f4c7 100644 --- a/util/error.c +++ b/util/error.c @@ -23,9 +23,11 @@ struct Error ErrorClass err_class; }; -void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...) +void error_set_bt(const char *file, const char *func, int line, + Error **errp, ErrorClass err_class, const char *fmt, ...) { Error *err; + char *msg1; va_list ap; if (errp == NULL) { @@ -36,15 +38,22 @@ void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...) err = g_malloc0(sizeof(*err)); va_start(ap, fmt); - err->msg = g_strdup_vprintf(fmt, ap); + msg1 = g_strdup_vprintf(fmt, ap); +#ifdef CONFIG_ERROR_BACKTRACE + err->msg = g_strdup_printf("%s:%i (in %s): %s", file, line, func, msg1); + g_free(msg1); +#else + err->msg = msg1; +#endif va_end(ap); err->err_class = err_class; *errp = err; } -void error_set_errno(Error **errp, int os_errno, ErrorClass err_class, - const char *fmt, ...) +void error_set_errno_bt(const char *file, const char *func, int line, + Error **errp, int os_errno, ErrorClass err_class, + const char *fmt, ...) { Error *err; char *msg1; @@ -59,21 +68,34 @@ void error_set_errno(Error **errp, int os_errno, ErrorClass err_class, va_start(ap, fmt); msg1 = g_strdup_vprintf(fmt, ap); +#ifdef CONFIG_ERROR_BACKTRACE + if (os_errno != 0) { + err->msg = g_strdup_printf("%s:%i (in %s): %s: %s", file, line, func, + msg1, strerror(os_errno)); + } else { + err->msg = g_strdup_printf("%s:%i (in %s): %s", file, line, func, msg1); + } + g_free(msg1); +#else if (os_errno != 0) { err->msg = g_strdup_printf("%s: %s", msg1, strerror(os_errno)); g_free(msg1); } else { err->msg = msg1; } +#endif va_end(ap); err->err_class = err_class; *errp = err; } -void error_setg_file_open(Error **errp, int os_errno, const char *filename) +void error_setg_file_open_bt(const char *file, const char *func, int line, + Error **errp, int os_errno, const char *filename) { - error_setg_errno(errp, os_errno, "Could not open '%s'", filename); + error_set_errno_bt(file, func, line, errp, os_errno, + ERROR_CLASS_GENERIC_ERROR, "Could not open '%s'", + filename); } Error *error_copy(const Error *err) @@ -110,11 +132,26 @@ void error_free(Error *err) } } -void error_propagate(Error **dst_err, Error *local_err) + +void error_propagate_bt(const char *file, const char *func, int line, + Error **dst_err, Error *local_err) { - if (dst_err && !*dst_err) { - *dst_err = local_err; - } else if (local_err) { + if (local_err) { +#ifdef CONFIG_ERROR_BACKTRACE + if (dst_err && !*dst_err) { + Error *err = g_malloc0(sizeof(*err)); + err->msg = g_strdup_printf("%s\n from %s:%i (in %s)", + local_err->msg, file, line, func); + err->err_class = local_err->err_class; + *dst_err = err; + } error_free(local_err); +#else + if (dst_err && !*dst_err) { + *dst_err = local_err; + } else { + error_free(local_err); + } +#endif } }
Add a configure switch which enables an error propagation backtrace. This results in the error_set function prepending every message by the source file name, function and line in which it was called, as well as error_propagate appending this information to the propagated message, resulting in a backtrace. Signed-off-by: Max Reitz <mreitz@redhat.com> --- Since this obviously breaks existing tests (and cannot really be used for new tests since it includes a line number, which is a rather volatile output) and is generally not very useful to normal users, the switch is disabled by default. This functionality is intended for developers tracking down error paths. Since I do not know whether I am the only one considering it useful in the first place, this is just an RFC for now. Furthermore, I am not sure whether a configure switch is really the right way to implement this. --- configure | 12 +++++++++++ include/qapi/error.h | 40 +++++++++++++++++++++++++++--------- util/error.c | 57 +++++++++++++++++++++++++++++++++++++++++++--------- 3 files changed, 90 insertions(+), 19 deletions(-)