| Message ID | 0b14290d85eceaa145e0333c26826b14304e23e4.1498761179.git.alistair.francis@xilinx.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 06/29/2017 02:42 PM, Alistair Francis wrote: > This patch removes the exisinting error_vreport() function and replaces it s/exisinting/existing/ > with a more generic vreport() function that takes an enum describing the > information to be reported. > > As part of this change a report() function is added as well with the > same capability. > > To maintain full compatibility the original error_report() function is > maintained and no changes to the way errors are printed have been made. > > Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> > --- > > -void error_vreport(const char *fmt, va_list ap) > +void vreport(report_types type, const char *fmt, va_list ap) > { > GTimeVal tv; > gchar *timestr; > > + switch (type) { > + case ERROR: > + /* To maintin compatibility we don't add anything here */ s/maintin/maintain/
On Thu, Jun 29, 2017 at 12:42:38PM -0700, Alistair Francis wrote: > This patch removes the exisinting error_vreport() function and replaces it > with a more generic vreport() function that takes an enum describing the > information to be reported. > > As part of this change a report() function is added as well with the > same capability. > > To maintain full compatibility the original error_report() function is > maintained and no changes to the way errors are printed have been made. > > Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> > --- > > hw/virtio/virtio.c | 2 +- > include/qemu/error-report.h | 10 +++++++++- > scripts/checkpatch.pl | 3 ++- > util/qemu-error.c | 33 ++++++++++++++++++++++++++++++--- > 4 files changed, 42 insertions(+), 6 deletions(-) > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > index 464947f76d..bd3d26abb7 100644 > --- a/hw/virtio/virtio.c > +++ b/hw/virtio/virtio.c > @@ -2448,7 +2448,7 @@ void GCC_FMT_ATTR(2, 3) virtio_error(VirtIODevice *vdev, const char *fmt, ...) > va_list ap; > > va_start(ap, fmt); > - error_vreport(fmt, ap); > + vreport(ERROR, fmt, ap); > va_end(ap); > > if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) { > diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h > index 3001865896..39b554c3b9 100644 > --- a/include/qemu/error-report.h > +++ b/include/qemu/error-report.h > @@ -21,6 +21,12 @@ typedef struct Location { > struct Location *prev; > } Location; > > +typedef enum { > + ERROR, > + WARN, > + INFO, > +} report_types; Woah, those are faaar to generic names to be used. There is way too much chance of those clashing with definitions from headers we pull in - particularly windows which pollutes its system headers with loads of generic names. I'd suggest QMSG_ERROR, QMSG_WARN, QMSG_INFO > + > Location *loc_push_restore(Location *loc); > Location *loc_push_none(Location *loc); > Location *loc_pop(Location *loc); > @@ -30,12 +36,14 @@ void loc_set_none(void); > void loc_set_cmdline(char **argv, int idx, int cnt); > void loc_set_file(const char *fname, int lno); > > +void vreport(report_types type, const char *fmt, va_list ap) GCC_FMT_ATTR(2, 0); > +void report(report_types type, const char *fmt, ...) GCC_FMT_ATTR(2, 3); Those names are too generic too IMHO. I'd suggest qmsg_report, qmsg_vreport As mentioned in the previous review, there should be wrappers which call these with suitable enum to make usage less verbose. eg qmsg_info(fmt, ....) should call qmsg_report(QMSG_INFO, fmt, ...) qmsg_vinfo(fmt, ....) should call qmsg_vreport(QMSG_INFO, fmt, ...) likewise, for other message levels > void error_vprintf(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0); > void error_printf(const char *fmt, ...) GCC_FMT_ATTR(1, 2); > void error_vprintf_unless_qmp(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0); > void error_printf_unless_qmp(const char *fmt, ...) GCC_FMT_ATTR(1, 2); > void error_set_progname(const char *argv0); > -void error_vreport(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0); > void error_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2); > const char *error_get_progname(void); > extern bool enable_timestamp_msg; Regards, Daniel
"Daniel P. Berrange" <berrange@redhat.com> writes: > On Thu, Jun 29, 2017 at 12:42:38PM -0700, Alistair Francis wrote: >> This patch removes the exisinting error_vreport() function and replaces it >> with a more generic vreport() function that takes an enum describing the >> information to be reported. Why remove error_vreport()? >> As part of this change a report() function is added as well with the >> same capability. >> >> To maintain full compatibility the original error_report() function is >> maintained and no changes to the way errors are printed have been made. >> >> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> >> --- >> >> hw/virtio/virtio.c | 2 +- >> include/qemu/error-report.h | 10 +++++++++- >> scripts/checkpatch.pl | 3 ++- >> util/qemu-error.c | 33 ++++++++++++++++++++++++++++++--- >> 4 files changed, 42 insertions(+), 6 deletions(-) >> >> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c >> index 464947f76d..bd3d26abb7 100644 >> --- a/hw/virtio/virtio.c >> +++ b/hw/virtio/virtio.c >> @@ -2448,7 +2448,7 @@ void GCC_FMT_ATTR(2, 3) virtio_error(VirtIODevice *vdev, const char *fmt, ...) >> va_list ap; >> >> va_start(ap, fmt); >> - error_vreport(fmt, ap); >> + vreport(ERROR, fmt, ap); >> va_end(ap); >> >> if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) { >> diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h >> index 3001865896..39b554c3b9 100644 >> --- a/include/qemu/error-report.h >> +++ b/include/qemu/error-report.h >> @@ -21,6 +21,12 @@ typedef struct Location { >> struct Location *prev; >> } Location; >> >> +typedef enum { >> + ERROR, >> + WARN, >> + INFO, >> +} report_types; > > Woah, those are faaar to generic names to be used. There is way too much > chance of those clashing with definitions from headers we pull in - particularly > windows which pollutes its system headers with loads of generic names. > > I'd suggest QMSG_ERROR, QMSG_WARN, QMSG_INFO > >> + >> Location *loc_push_restore(Location *loc); >> Location *loc_push_none(Location *loc); >> Location *loc_pop(Location *loc); >> @@ -30,12 +36,14 @@ void loc_set_none(void); >> void loc_set_cmdline(char **argv, int idx, int cnt); >> void loc_set_file(const char *fname, int lno); >> >> +void vreport(report_types type, const char *fmt, va_list ap) GCC_FMT_ATTR(2, 0); >> +void report(report_types type, const char *fmt, ...) GCC_FMT_ATTR(2, 3); > > Those names are too generic too IMHO. I'd suggest qmsg_report, qmsg_vreport > > As mentioned in the previous review, there should be wrappers which > call these with suitable enum to make usage less verbose. eg > > qmsg_info(fmt, ....) should call qmsg_report(QMSG_INFO, fmt, ...) > qmsg_vinfo(fmt, ....) should call qmsg_vreport(QMSG_INFO, fmt, ...) > > likewise, for other message levels We then have qmsg_warning() for warnings, and error_report() for errors. Ugh! If I had known back then what I know now, I wouldn't have used the error_ prefix. Naming things is hard. Ideas anyone? >> void error_vprintf(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0); >> void error_printf(const char *fmt, ...) GCC_FMT_ATTR(1, 2); >> void error_vprintf_unless_qmp(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0); >> void error_printf_unless_qmp(const char *fmt, ...) GCC_FMT_ATTR(1, 2); >> void error_set_progname(const char *argv0); >> -void error_vreport(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0); >> void error_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2); >> const char *error_get_progname(void); >> extern bool enable_timestamp_msg; > > Regards, > Daniel
On Mon, Jul 03, 2017 at 04:07:21PM +0200, Markus Armbruster wrote: > "Daniel P. Berrange" <berrange@redhat.com> writes: > > > On Thu, Jun 29, 2017 at 12:42:38PM -0700, Alistair Francis wrote: > >> This patch removes the exisinting error_vreport() function and replaces it > >> with a more generic vreport() function that takes an enum describing the > >> information to be reported. > > Why remove error_vreport()? > > >> As part of this change a report() function is added as well with the > >> same capability. > >> > >> To maintain full compatibility the original error_report() function is > >> maintained and no changes to the way errors are printed have been made. > >> > >> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> > >> --- > >> > >> hw/virtio/virtio.c | 2 +- > >> include/qemu/error-report.h | 10 +++++++++- > >> scripts/checkpatch.pl | 3 ++- > >> util/qemu-error.c | 33 ++++++++++++++++++++++++++++++--- > >> 4 files changed, 42 insertions(+), 6 deletions(-) > >> > >> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > >> index 464947f76d..bd3d26abb7 100644 > >> --- a/hw/virtio/virtio.c > >> +++ b/hw/virtio/virtio.c > >> @@ -2448,7 +2448,7 @@ void GCC_FMT_ATTR(2, 3) virtio_error(VirtIODevice *vdev, const char *fmt, ...) > >> va_list ap; > >> > >> va_start(ap, fmt); > >> - error_vreport(fmt, ap); > >> + vreport(ERROR, fmt, ap); > >> va_end(ap); > >> > >> if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) { > >> diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h > >> index 3001865896..39b554c3b9 100644 > >> --- a/include/qemu/error-report.h > >> +++ b/include/qemu/error-report.h > >> @@ -21,6 +21,12 @@ typedef struct Location { > >> struct Location *prev; > >> } Location; > >> > >> +typedef enum { > >> + ERROR, > >> + WARN, > >> + INFO, > >> +} report_types; > > > > Woah, those are faaar to generic names to be used. There is way too much > > chance of those clashing with definitions from headers we pull in - particularly > > windows which pollutes its system headers with loads of generic names. > > > > I'd suggest QMSG_ERROR, QMSG_WARN, QMSG_INFO > > > >> + > >> Location *loc_push_restore(Location *loc); > >> Location *loc_push_none(Location *loc); > >> Location *loc_pop(Location *loc); > >> @@ -30,12 +36,14 @@ void loc_set_none(void); > >> void loc_set_cmdline(char **argv, int idx, int cnt); > >> void loc_set_file(const char *fname, int lno); > >> > >> +void vreport(report_types type, const char *fmt, va_list ap) GCC_FMT_ATTR(2, 0); > >> +void report(report_types type, const char *fmt, ...) GCC_FMT_ATTR(2, 3); > > > > Those names are too generic too IMHO. I'd suggest qmsg_report, qmsg_vreport > > > > As mentioned in the previous review, there should be wrappers which > > call these with suitable enum to make usage less verbose. eg > > > > qmsg_info(fmt, ....) should call qmsg_report(QMSG_INFO, fmt, ...) > > qmsg_vinfo(fmt, ....) should call qmsg_vreport(QMSG_INFO, fmt, ...) > > > > likewise, for other message levels > > We then have qmsg_warning() for warnings, and error_report() for errors. > Ugh! > > If I had known back then what I know now, I wouldn't have used the > error_ prefix. > > Naming things is hard. > > Ideas anyone? I guess implicit in my suggestion would be to switch to qmsg_error() over some (long) period of time, but that would be a massive amount of churn that would harm backporting. So perhaps, just have error_report warning_report, info_report, and accept that the naming convention is slightly reversed from "normality" Regards, Daniel
"Daniel P. Berrange" <berrange@redhat.com> writes: > On Mon, Jul 03, 2017 at 04:07:21PM +0200, Markus Armbruster wrote: >> "Daniel P. Berrange" <berrange@redhat.com> writes: >> >> > On Thu, Jun 29, 2017 at 12:42:38PM -0700, Alistair Francis wrote: >> >> This patch removes the exisinting error_vreport() function and replaces it >> >> with a more generic vreport() function that takes an enum describing the >> >> information to be reported. >> >> Why remove error_vreport()? >> >> >> As part of this change a report() function is added as well with the >> >> same capability. >> >> >> >> To maintain full compatibility the original error_report() function is >> >> maintained and no changes to the way errors are printed have been made. >> >> >> >> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> >> >> --- >> >> >> >> hw/virtio/virtio.c | 2 +- >> >> include/qemu/error-report.h | 10 +++++++++- >> >> scripts/checkpatch.pl | 3 ++- >> >> util/qemu-error.c | 33 ++++++++++++++++++++++++++++++--- >> >> 4 files changed, 42 insertions(+), 6 deletions(-) >> >> >> >> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c >> >> index 464947f76d..bd3d26abb7 100644 >> >> --- a/hw/virtio/virtio.c >> >> +++ b/hw/virtio/virtio.c >> >> @@ -2448,7 +2448,7 @@ void GCC_FMT_ATTR(2, 3) virtio_error(VirtIODevice *vdev, const char *fmt, ...) >> >> va_list ap; >> >> >> >> va_start(ap, fmt); >> >> - error_vreport(fmt, ap); >> >> + vreport(ERROR, fmt, ap); >> >> va_end(ap); >> >> >> >> if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) { >> >> diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h >> >> index 3001865896..39b554c3b9 100644 >> >> --- a/include/qemu/error-report.h >> >> +++ b/include/qemu/error-report.h >> >> @@ -21,6 +21,12 @@ typedef struct Location { >> >> struct Location *prev; >> >> } Location; >> >> >> >> +typedef enum { >> >> + ERROR, >> >> + WARN, >> >> + INFO, >> >> +} report_types; >> > >> > Woah, those are faaar to generic names to be used. There is way too much >> > chance of those clashing with definitions from headers we pull in - particularly >> > windows which pollutes its system headers with loads of generic names. >> > >> > I'd suggest QMSG_ERROR, QMSG_WARN, QMSG_INFO >> > >> >> + >> >> Location *loc_push_restore(Location *loc); >> >> Location *loc_push_none(Location *loc); >> >> Location *loc_pop(Location *loc); >> >> @@ -30,12 +36,14 @@ void loc_set_none(void); >> >> void loc_set_cmdline(char **argv, int idx, int cnt); >> >> void loc_set_file(const char *fname, int lno); >> >> >> >> +void vreport(report_types type, const char *fmt, va_list ap) GCC_FMT_ATTR(2, 0); >> >> +void report(report_types type, const char *fmt, ...) GCC_FMT_ATTR(2, 3); >> > >> > Those names are too generic too IMHO. I'd suggest qmsg_report, qmsg_vreport >> > >> > As mentioned in the previous review, there should be wrappers which >> > call these with suitable enum to make usage less verbose. eg >> > >> > qmsg_info(fmt, ....) should call qmsg_report(QMSG_INFO, fmt, ...) >> > qmsg_vinfo(fmt, ....) should call qmsg_vreport(QMSG_INFO, fmt, ...) >> > >> > likewise, for other message levels >> >> We then have qmsg_warning() for warnings, and error_report() for errors. >> Ugh! >> >> If I had known back then what I know now, I wouldn't have used the >> error_ prefix. >> >> Naming things is hard. >> >> Ideas anyone? > > I guess implicit in my suggestion would be to switch to qmsg_error() > over some (long) period of time, but that would be a massive amount > of churn that would harm backporting. > > So perhaps, just have error_report warning_report, info_report, and > accept that the naming convention is slightly reversed from "normality" That's not half bad. The matching enum would be typedef enum { REPORT_TYPE_ERROR, REPORT_TYPE_WARNING, REPORT_TYPE_INFO, } report_type; Note the typedef name is *singular*. Compare report_type rtype; to report_types rtype; @rtype is *one* report type, not multiple. Let's stick report_type, report() and vreport() into qemu-error.c (static linkage) until we have a genuine need for them elsewhere.
On Mon, Jul 3, 2017 at 11:53 PM, Markus Armbruster <armbru@redhat.com> wrote: > "Daniel P. Berrange" <berrange@redhat.com> writes: > >> On Mon, Jul 03, 2017 at 04:07:21PM +0200, Markus Armbruster wrote: >>> "Daniel P. Berrange" <berrange@redhat.com> writes: >>> >>> > On Thu, Jun 29, 2017 at 12:42:38PM -0700, Alistair Francis wrote: >>> >> This patch removes the exisinting error_vreport() function and replaces it >>> >> with a more generic vreport() function that takes an enum describing the >>> >> information to be reported. >>> >>> Why remove error_vreport()? I just thought that the name didn't make sense now that we are using it for info and warning reports as well. >>> >>> >> As part of this change a report() function is added as well with the >>> >> same capability. >>> >> >>> >> To maintain full compatibility the original error_report() function is >>> >> maintained and no changes to the way errors are printed have been made. >>> >> >>> >> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> >>> >> --- >>> >> >>> >> hw/virtio/virtio.c | 2 +- >>> >> include/qemu/error-report.h | 10 +++++++++- >>> >> scripts/checkpatch.pl | 3 ++- >>> >> util/qemu-error.c | 33 ++++++++++++++++++++++++++++++--- >>> >> 4 files changed, 42 insertions(+), 6 deletions(-) >>> >> >>> >> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c >>> >> index 464947f76d..bd3d26abb7 100644 >>> >> --- a/hw/virtio/virtio.c >>> >> +++ b/hw/virtio/virtio.c >>> >> @@ -2448,7 +2448,7 @@ void GCC_FMT_ATTR(2, 3) virtio_error(VirtIODevice *vdev, const char *fmt, ...) >>> >> va_list ap; >>> >> >>> >> va_start(ap, fmt); >>> >> - error_vreport(fmt, ap); >>> >> + vreport(ERROR, fmt, ap); >>> >> va_end(ap); >>> >> >>> >> if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) { >>> >> diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h >>> >> index 3001865896..39b554c3b9 100644 >>> >> --- a/include/qemu/error-report.h >>> >> +++ b/include/qemu/error-report.h >>> >> @@ -21,6 +21,12 @@ typedef struct Location { >>> >> struct Location *prev; >>> >> } Location; >>> >> >>> >> +typedef enum { >>> >> + ERROR, >>> >> + WARN, >>> >> + INFO, >>> >> +} report_types; >>> > >>> > Woah, those are faaar to generic names to be used. There is way too much >>> > chance of those clashing with definitions from headers we pull in - particularly >>> > windows which pollutes its system headers with loads of generic names. >>> > >>> > I'd suggest QMSG_ERROR, QMSG_WARN, QMSG_INFO >>> > >>> >> + >>> >> Location *loc_push_restore(Location *loc); >>> >> Location *loc_push_none(Location *loc); >>> >> Location *loc_pop(Location *loc); >>> >> @@ -30,12 +36,14 @@ void loc_set_none(void); >>> >> void loc_set_cmdline(char **argv, int idx, int cnt); >>> >> void loc_set_file(const char *fname, int lno); >>> >> >>> >> +void vreport(report_types type, const char *fmt, va_list ap) GCC_FMT_ATTR(2, 0); >>> >> +void report(report_types type, const char *fmt, ...) GCC_FMT_ATTR(2, 3); >>> > >>> > Those names are too generic too IMHO. I'd suggest qmsg_report, qmsg_vreport Ok, I was trying to make them as short as possible to allow less line wrapping, but I agree report() is too generic. >>> > >>> > As mentioned in the previous review, there should be wrappers which >>> > call these with suitable enum to make usage less verbose. eg >>> > >>> > qmsg_info(fmt, ....) should call qmsg_report(QMSG_INFO, fmt, ...) >>> > qmsg_vinfo(fmt, ....) should call qmsg_vreport(QMSG_INFO, fmt, ...) >>> > >>> > likewise, for other message levels >>> >>> We then have qmsg_warning() for warnings, and error_report() for errors. >>> Ugh! >>> >>> If I had known back then what I know now, I wouldn't have used the >>> error_ prefix. >>> >>> Naming things is hard. >>> >>> Ideas anyone? >> >> I guess implicit in my suggestion would be to switch to qmsg_error() >> over some (long) period of time, but that would be a massive amount >> of churn that would harm backporting. >> >> So perhaps, just have error_report warning_report, info_report, and >> accept that the naming convention is slightly reversed from "normality" > > That's not half bad. > > The matching enum would be > > typedef enum { > REPORT_TYPE_ERROR, > REPORT_TYPE_WARNING, > REPORT_TYPE_INFO, > } report_type; > > Note the typedef name is *singular*. Compare > > report_type rtype; > > to > > report_types rtype; > > @rtype is *one* report type, not multiple. > > Let's stick report_type, report() and vreport() into qemu-error.c > (static linkage) until we have a genuine need for them elsewhere. Ok, I'll send out another version later today then. Thanks, Alistair
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 464947f76d..bd3d26abb7 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -2448,7 +2448,7 @@ void GCC_FMT_ATTR(2, 3) virtio_error(VirtIODevice *vdev, const char *fmt, ...) va_list ap; va_start(ap, fmt); - error_vreport(fmt, ap); + vreport(ERROR, fmt, ap); va_end(ap); if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) { diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h index 3001865896..39b554c3b9 100644 --- a/include/qemu/error-report.h +++ b/include/qemu/error-report.h @@ -21,6 +21,12 @@ typedef struct Location { struct Location *prev; } Location; +typedef enum { + ERROR, + WARN, + INFO, +} report_types; + Location *loc_push_restore(Location *loc); Location *loc_push_none(Location *loc); Location *loc_pop(Location *loc); @@ -30,12 +36,14 @@ void loc_set_none(void); void loc_set_cmdline(char **argv, int idx, int cnt); void loc_set_file(const char *fname, int lno); +void vreport(report_types type, const char *fmt, va_list ap) GCC_FMT_ATTR(2, 0); +void report(report_types type, const char *fmt, ...) GCC_FMT_ATTR(2, 3); + void error_vprintf(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0); void error_printf(const char *fmt, ...) GCC_FMT_ATTR(1, 2); void error_vprintf_unless_qmp(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0); void error_printf_unless_qmp(const char *fmt, ...) GCC_FMT_ATTR(1, 2); void error_set_progname(const char *argv0); -void error_vreport(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0); void error_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2); const char *error_get_progname(void); extern bool enable_timestamp_msg; diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 45027b9281..4a3074e758 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2530,7 +2530,8 @@ sub process { error_set| error_prepend| error_reportf_err| - error_vreport| + vreport| + report| error_report}x; if ($rawline =~ /\b(?:$qemu_error_funcs)\s*\(.*\".*\\n/) { diff --git a/util/qemu-error.c b/util/qemu-error.c index 1c5e35ecdb..83206532dd 100644 --- a/util/qemu-error.c +++ b/util/qemu-error.c @@ -179,17 +179,28 @@ static void print_loc(void) bool enable_timestamp_msg; /* - * Print an error message to current monitor if we have one, else to stderr. + * Print a message to current monitor if we have one, else to stderr. * Format arguments like vsprintf(). The resulting message should be * a single phrase, with no newline or trailing punctuation. * Prepend the current location and append a newline. * It's wrong to call this in a QMP monitor. Use error_setg() there. */ -void error_vreport(const char *fmt, va_list ap) +void vreport(report_types type, const char *fmt, va_list ap) { GTimeVal tv; gchar *timestr; + switch (type) { + case ERROR: + /* To maintin compatibility we don't add anything here */ + break; + case WARN: + error_printf("warning: "); + break; + case INFO: + error_printf("info: "); + break; + } if (enable_timestamp_msg && !cur_mon) { g_get_current_time(&tv); timestr = g_time_val_to_iso8601(&tv); @@ -203,6 +214,22 @@ void error_vreport(const char *fmt, va_list ap) } /* + * Print a message to current monitor if we have one, else to stderr. + * Format arguments like sprintf(). The resulting message should be a + * single phrase, with no newline or trailing punctuation. + * Prepend the current location and append a newline. + * It's wrong to call this in a QMP monitor. Use error_setg() there. + */ +void report(report_types type, const char *fmt, ...) +{ + va_list ap; + + va_start(ap, fmt); + vreport(type, fmt, ap); + va_end(ap); +} + +/* * Print an error message to current monitor if we have one, else to stderr. * Format arguments like sprintf(). The resulting message should be a * single phrase, with no newline or trailing punctuation. @@ -214,6 +241,6 @@ void error_report(const char *fmt, ...) va_list ap; va_start(ap, fmt); - error_vreport(fmt, ap); + vreport(ERROR, fmt, ap); va_end(ap); }
This patch removes the exisinting error_vreport() function and replaces it with a more generic vreport() function that takes an enum describing the information to be reported. As part of this change a report() function is added as well with the same capability. To maintain full compatibility the original error_report() function is maintained and no changes to the way errors are printed have been made. Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> --- hw/virtio/virtio.c | 2 +- include/qemu/error-report.h | 10 +++++++++- scripts/checkpatch.pl | 3 ++- util/qemu-error.c | 33 ++++++++++++++++++++++++++++++--- 4 files changed, 42 insertions(+), 6 deletions(-)