On 11/08/2017 04:56 PM, Alistair Francis wrote: > Replace a large number of the fprintf(stderr, "*\n" calls with > error_report(). The functions were renamed with these commands and then > compiler issues where manually fixed.
s/where/were/ > > Some of the error_report()'s were manually kept as fprintf(stderr, ) to > maintain standalone test cases. > > Signed-off-by: Alistair Francis <alistair.fran...@xilinx.com> > Cc: "Michael S. Tsirkin" <m...@redhat.com> > Cc: Igor Mammedov <imamm...@redhat.com> > Cc: "Dr. David Alan Gilbert" <dgilb...@redhat.com> > Cc: Gerd Hoffmann <kra...@redhat.com> > Cc: qemu-bl...@nongnu.org > --- > V2: > - Keep some of the fprintf() calls > - Remove a file I accidently checked in > > +++ b/tests/ahci-test.c > @@ -23,6 +23,7 @@ > */ > > #include "qemu/osdep.h" > +#include "qemu/error-report.h" > #include <getopt.h> > > #include "libqtest.h" > @@ -1859,7 +1860,7 @@ int main(int argc, char **argv) > ahci_pedantic = 1; > break; > default: > - fprintf(stderr, "Unrecognized ahci_test option.\n"); > + error_report("Unrecognized ahci_test option."); Most of our error_report() calls do not include trailing dot. > +++ b/tests/bios-tables-test.c > @@ -11,6 +11,7 @@ > */ > > #include "qemu/osdep.h" > +#include "qemu/error-report.h" > #include <glib/gstdio.h> > #include "qemu-common.h" > #include "hw/smbios/smbios.h" > @@ -396,7 +397,7 @@ try_again: > aml_file = g_strdup_printf("%s/%s/%.4s%s", data_dir, data->machine, > (gchar *)&signature, ext); > if (getenv("V")) { > - fprintf(stderr, "\nLooking for expected file '%s'\n", aml_file); > + error_report("Looking for expected file '%s'", aml_file); Here, it looks like this is a debug statement, gated by whether $V is set in the environment; it's not really an error. > } > if (g_file_test(aml_file, G_FILE_TEST_EXISTS)) { > exp_sdt.aml_file = aml_file; > @@ -408,7 +409,7 @@ try_again: > } > g_assert(exp_sdt.aml_file); > if (getenv("V")) { > - fprintf(stderr, "\nUsing expected file '%s'\n", aml_file); > + error_report("Using expected file '%s'", aml_file); > } Ditto. > +++ b/tests/i440fx-test.c > @@ -13,7 +13,7 @@ > */ > > #include "qemu/osdep.h" > - > +#include "qemu/error-report.h" > #include "libqtest.h" > #include "libqos/pci.h" > #include "libqos/pci-pc.h" > @@ -295,18 +295,18 @@ static char *create_blob_file(void) > ret = -1; > fd = g_file_open_tmp("blob_XXXXXX", &pathname, &error); > if (fd == -1) { > - fprintf(stderr, "unable to create blob file: %s\n", error->message); > + error_report("unable to create blob file: %s", error->message); > g_error_free(error); A candidate for error_reportf_err(error, "unable to create blob file: "); > +++ b/tests/libqos/ahci.c > @@ -23,7 +23,7 @@ > */ > > #include "qemu/osdep.h" > - > +#include "qemu/error-report.h" > #include "libqtest.h" > #include "libqos/ahci.h" > #include "libqos/pci-pc.h" > @@ -985,9 +985,9 @@ static void ahci_atapi_command_set_offset(AHCICommand > *cmd, uint64_t lba) > default: > /* SCSI doesn't have uniform packet formats, > * so you have to add support for it manually. Sorry! */ > - fprintf(stderr, "The Libqos AHCI driver does not support the " > + error_report("The Libqos AHCI driver does not support the " > "set_offset operation for ATAPI command 0x%02x, " > - "please add support.\n", > + "please add support.", Trailing dot is unusual. > cbd[0]); > g_assert_not_reached(); > } > @@ -1060,8 +1060,8 @@ static void ahci_atapi_set_size(AHCICommand *cmd, > uint64_t xbytes) > default: > /* SCSI doesn't have uniform packet formats, > * so you have to add support for it manually. Sorry! */ > - fprintf(stderr, "The Libqos AHCI driver does not support the > set_size " > - "operation for ATAPI command 0x%02x, please add support.\n", > + error_report("The Libqos AHCI driver does not support the set_size " > + "operation for ATAPI command 0x%02x, please add support.", Again > +++ b/tests/libqos/libqos.c > @@ -199,7 +200,7 @@ void mkimg(const char *file, const char *fmt, unsigned > size_mb) > fmt, file, size_mb); > ret = g_spawn_command_line_sync(cli, &out, &out2, &rc, &err); > if (err) { > - fprintf(stderr, "%s\n", err->message); > + error_report("%s", err->message); > g_error_free(err); Candidate for error_report_err() > +++ b/tests/libqos/malloc.c > @@ -11,6 +11,7 @@ > */ > > #include "qemu/osdep.h" > +#include "qemu/error-report.h" > #include "libqos/malloc.h" > #include "qemu-common.h" > #include "qemu/host-utils.h" > @@ -193,7 +194,7 @@ static uint64_t mlist_alloc(QGuestAllocator *s, uint64_t > size) > > node = mlist_find_space(s->free, size); > if (!node) { > - fprintf(stderr, "Out of guest memory.\n"); > + error_report("Out of guest memory."); trailing dot > g_assert_not_reached(); > } > return mlist_fulfill(s, node, size); > @@ -209,8 +210,8 @@ static void mlist_free(QGuestAllocator *s, uint64_t addr) > > node = mlist_find_key(s->used, addr); > if (!node) { > - fprintf(stderr, "Error: no record found for an allocation at " > - "0x%016" PRIx64 ".\n", > + error_report("Error: no record found for an allocation at " > + "0x%016" PRIx64 ".", Again. I'll quit pointing these out. > +++ b/tests/migration-test.c > @@ -334,9 +334,9 @@ static void check_guests_ram(QTestState *who) > */ > hit_edge = true; > } else { > - fprintf(stderr, "Memory content inconsistency at %x" > + error_report("Memory content inconsistency at %x" > " first_byte = %x last_byte = %x current = > %x" > - " hit_edge = %x\n", > + " hit_edge = %x", Indentation is now off. > address, first_byte, last_byte, b, hit_edge); > bad = true; > } > diff --git a/tests/migration/stress.c b/tests/migration/stress.c > index cf8ce8b16d..49e1ff4555 100644 > --- a/tests/migration/stress.c > +++ b/tests/migration/stress.c > @@ -47,7 +47,7 @@ static __attribute__((noreturn)) void exit_failure(void) > if (getpid() == 1) { > sync(); > reboot(RB_POWER_OFF); > - fprintf(stderr, "%s (%05d): ERROR: cannot reboot: %s\n", > + error_report("%s (%05d): cannot reboot: %s", > argv0, gettid(), strerror(errno)); Indentation is now off. > abort(); > } else { > @@ -60,7 +60,7 @@ static __attribute__((noreturn)) void exit_success(void) > if (getpid() == 1) { > sync(); > reboot(RB_POWER_OFF); > - fprintf(stderr, "%s (%05d): ERROR: cannot reboot: %s\n", > + error_report("%s (%05d): cannot reboot: %s", > argv0, gettid(), strerror(errno)); And again. I'll quit pointing it out. > +++ b/tests/rcutorture.c > @@ -61,6 +61,7 @@ > */ > > #include "qemu/osdep.h" > +#include "qemu/error-report.h" > #include "qemu/atomic.h" > #include "qemu/rcu.h" > #include "qemu/thread.h" > @@ -86,7 +87,7 @@ static int n_threads; > static void create_thread(void *(*func)(void *)) > { > if (n_threads >= NR_THREADS) { > - fprintf(stderr, "Thread limit of %d exceeded!\n", NR_THREADS); > + error_report("Thread limit of %d exceeded!", NR_THREADS); Trailing ! is shouting at the user; it's even less appropriate than trailing dot. > exit(-1); > } > qemu_thread_create(&threads[n_threads], "test", func, &data[n_threads], > @@ -417,7 +418,7 @@ static void gtest_stress_10_5(void) > > static void usage(int argc, char *argv[]) > { > - fprintf(stderr, "Usage: %s [nreaders [ perf | stress ] ]\n", argv[0]); > + error_report("Usage: %s [nreaders [ perf | stress ] ]", argv[0]); > exit(-1); Separate patch - but 'exit(-1)' is almost always wrong (it gives status 255 through wait()/waitpid(); meanwhile waitid() is required by POSIX to get at the full 32-bit value except that Linux doesn't obey that requirement). A process where wait() returns 255 makes xargs behave differently. We really want exit(1). > +++ b/tests/tcg/linux-test.c > @@ -51,7 +51,7 @@ void error1(const char *filename, int line, const char > *fmt, ...) > va_start(ap, fmt); > fprintf(stderr, "%s:%d: ", filename, line); > vfprintf(stderr, fmt, ap); > - fprintf(stderr, "\n"); > + error_report(""); Umm, a blank line is not a useful error. This hunk is bogus; we probably want to stick with fprintf() for the entire message. > +++ b/tests/tcg/test_path.c > @@ -150,8 +150,8 @@ int main(int argc, char *argv[]) > ret = do_test(); > cleanup(); > if (ret) { > - fprintf(stderr, "test_path: failed on line %i\n", ret); > - return 1; > + error_report("test_path: failed on line %i", ret); > + return 1; Yay for fixing TAB damage along the way. > +++ b/tests/test-hmp.c > @@ -15,6 +15,7 @@ > */ > > #include "qemu/osdep.h" > +#include "qemu/error-report.h" > #include "libqtest.h" > > static int verbose; > @@ -79,7 +80,7 @@ static void test_commands(void) > > for (i = 0; hmp_cmds[i] != NULL; i++) { > if (verbose) { > - fprintf(stderr, "\t%s\n", hmp_cmds[i]); > + error_report("\t%s", hmp_cmds[i]); Verbose messages aren't really errors. I don't think you want this hunk. > } > response = hmp("%s", hmp_cmds[i]); > g_free(response); > @@ -102,7 +103,7 @@ static void test_info_commands(void) > *endp = '\0'; > /* Now run the info command */ > if (verbose) { > - fprintf(stderr, "\t%s\n", info); > + error_report("\t%s", info); Likewise. > +++ b/tests/test-rcu-list.c > @@ -21,6 +21,7 @@ > */ > > #include "qemu/osdep.h" > +#include "qemu/error-report.h" > #include "qemu/atomic.h" > #include "qemu/rcu.h" > #include "qemu/thread.h" > @@ -64,7 +65,7 @@ static int select_random_el(int max) > static void create_thread(void *(*func)(void *)) > { > if (n_threads >= NR_THREADS) { > - fprintf(stderr, "Thread limit of %d exceeded!\n", NR_THREADS); > + error_report("Thread limit of %d exceeded!", NR_THREADS); > exit(-1); Another !, another exit(-1) > +++ b/tests/usb-hcd-ehci-test.c > @@ -42,7 +42,7 @@ static void ehci_port_test(struct qhc *hc, int port, > uint32_t expect) > uint16_t mask = ~(PORTSC_CSC | PORTSC_PEDC | PORTSC_OCC); > > #if 0 > - fprintf(stderr, "%s: %d, have 0x%08x, want 0x%08x\n", > + error_report("%s: %d, have 0x%08x, want 0x%08x", > __func__, port, value & mask, expect & mask); > #endif I'd just nuke the #if 0 block entirely, as it is likely to bit-rot (separate patch, though). > +++ b/tests/vhost-user-bridge.c > @@ -714,15 +714,15 @@ main(int argc, char *argv[]) > return 0; > > out: > - fprintf(stderr, "Usage: %s ", argv[0]); > - fprintf(stderr, "[-c] [-u ud_socket_path] [-l lhost:lport] [-r > rhost:rport]\n"); > - fprintf(stderr, "\t-u path to unix doman socket. default: %s\n", > + error_report("Usage: %s ", argv[0]); > + error_report("[-c] [-u ud_socket_path] [-l lhost:lport] [-r > rhost:rport]"); > + error_report("\t-u path to unix doman socket. default: %s", > DEFAULT_UD_SOCKET); > - fprintf(stderr, "\t-l local host and port. default: %s:%s\n", > + fprintf(stderr, "\t-l local host and port. default: %s:%s", > DEFAULT_LHOST, DEFAULT_LPORT); > - fprintf(stderr, "\t-r remote host and port. default: %s:%s\n", > + error_report("\t-r remote host and port. default: %s:%s", > DEFAULT_RHOST, DEFAULT_RPORT); > - fprintf(stderr, "\t-c client mode\n"); > + error_report("\t-c client mode"); Is usage text really an error? > > return 1; > } > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature