[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 2/6] util: Replace fprintf(stderr, "*\n" with error_report
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v2 2/6] util: Replace fprintf(stderr, "*\n" with error_report() |
Date: |
Fri, 28 Feb 2020 08:43:13 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) |
Philippe Mathieu-Daudé <address@hidden> writes:
> From: Alistair Francis <address@hidden>
>
> 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.
>
> find ./* -type f -exec sed -i \
> 'N;N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr,
> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr,
> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N;N;N;N;N;N;N;N; {s|fprintf(stderr,
> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N;N;N;N;N;N;N; {s|fprintf(stderr,
> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N;N;N;N;N;N; {s|fprintf(stderr,
> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N;N;N;N;N; {s|fprintf(stderr,
> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N;N;N;N; {s|fprintf(stderr,
> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N;N;N; {s|fprintf(stderr,
> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}'
> \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
>
> The error in aio_poll() was removed manually.
>
> Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
> Message-Id: <address@hidden>
> Signed-off-by: Alistair Francis <address@hidden>
> [PMD: Rebased]
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> ---
> Cc: Alistair Francis <address@hidden>
> Cc: Alistair Francis <address@hidden>
> ---
> util/coroutine-sigaltstack.c | 3 ++-
> util/mmap-alloc.c | 11 ++++++-----
> util/module.c | 13 ++++++-------
> util/osdep.c | 4 ++--
> util/oslib-posix.c | 3 ++-
> util/oslib-win32.c | 3 ++-
> util/qemu-coroutine.c | 10 +++++-----
> util/qemu-thread-posix.c | 5 +++--
> util/qemu-thread-win32.c | 5 +++--
> util/qemu-timer-common.c | 3 ++-
> 10 files changed, 33 insertions(+), 27 deletions(-)
>
> diff --git a/util/coroutine-sigaltstack.c b/util/coroutine-sigaltstack.c
> index f6fc49a0e5..63decd4d1d 100644
> --- a/util/coroutine-sigaltstack.c
> +++ b/util/coroutine-sigaltstack.c
> @@ -29,6 +29,7 @@
> #include <pthread.h>
> #include "qemu-common.h"
> #include "qemu/coroutine_int.h"
> +#include "qemu/error-report.h"
>
> typedef struct {
> Coroutine base;
> @@ -80,7 +81,7 @@ static void __attribute__((constructor))
> coroutine_init(void)
>
> ret = pthread_key_create(&thread_state_key,
> qemu_coroutine_thread_cleanup);
> if (ret != 0) {
> - fprintf(stderr, "unable to create leader key: %s\n",
> strerror(errno));
> + error_report("unable to create leader key: %s", strerror(errno));
> abort();
> }
> }
> diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
> index 27dcccd8ec..3ac6e10404 100644
> --- a/util/mmap-alloc.c
> +++ b/util/mmap-alloc.c
> @@ -18,6 +18,7 @@
> #endif /* CONFIG_LINUX */
>
> #include "qemu/osdep.h"
> +#include "qemu/error-report.h"
> #include "qemu/mmap-alloc.h"
> #include "qemu/host-utils.h"
>
> @@ -63,7 +64,7 @@ size_t qemu_mempath_getpagesize(const char *mem_path)
> } while (ret != 0 && errno == EINTR);
>
> if (ret != 0) {
> - fprintf(stderr, "Couldn't statfs() memory path: %s\n",
> + error_report("Couldn't statfs() memory path: %s",
> strerror(errno));
Indentation is off.
> exit(1);
> }
> @@ -160,10 +161,10 @@ void *qemu_ram_mmap(int fd,
> len = 0;
> }
> file_name[len] = '\0';
> - fprintf(stderr, "Warning: requesting persistence across crashes "
> - "for backend file %s failed. Proceeding without "
> - "persistence, data might become corrupted in case of
> host "
> - "crash.\n", file_name);
> + error_report("Warning: requesting persistence across crashes "
> + "for backend file %s failed. Proceeding without "
> + "persistence, data might become corrupted in case "
> + "of host crash.", file_name);
This should be something like
warn_report("requesting persistence across crashes"
" for backend file %s failed",
file_name);
error_printf("Proceeding without persistence, data might"
" become corrupted in case of host crash.\n");
Precedence: commit db0754df88 "file-posix: Use error API properly".
> g_free(proc_link);
> g_free(file_name);
> }
> diff --git a/util/module.c b/util/module.c
> index 236a7bb52a..28efa1f891 100644
> --- a/util/module.c
> +++ b/util/module.c
> @@ -19,6 +19,7 @@
> #endif
> #include "qemu/queue.h"
> #include "qemu/module.h"
> +#include "qemu/error-report.h"
>
> typedef struct ModuleEntry
> {
> @@ -130,19 +131,17 @@ static int module_load_file(const char *fname)
>
> g_module = g_module_open(fname, G_MODULE_BIND_LAZY |
> G_MODULE_BIND_LOCAL);
> if (!g_module) {
> - fprintf(stderr, "Failed to open module: %s\n",
> - g_module_error());
> + error_report("Failed to open module: %s", g_module_error());
> ret = -EINVAL;
> goto out;
> }
> if (!g_module_symbol(g_module, DSO_STAMP_FUN_STR, (gpointer *)&sym)) {
> - fprintf(stderr, "Failed to initialize module: %s\n",
> - fname);
> + error_report("Failed to initialize module: %s", fname);
> /* Print some info if this is a QEMU module (but from different
> build),
> * this will make debugging user problems easier. */
> if (g_module_symbol(g_module, "qemu_module_dummy", (gpointer
> *)&sym)) {
> - fprintf(stderr,
> - "Note: only modules from the same build can be
> loaded.\n");
> + error_report("Note: "
> + "only modules from the same build can be loaded.");
Use error_printf() to print the additional note.
> }
> g_module_close(g_module);
> ret = -EINVAL;
> @@ -178,7 +177,7 @@ bool module_load_one(const char *prefix, const char
> *lib_name)
> static GHashTable *loaded_modules;
>
> if (!g_module_supported()) {
> - fprintf(stderr, "Module is not supported by system.\n");
> + error_report("Module is not supported by system.");
> return false;
> }
>
> diff --git a/util/osdep.c b/util/osdep.c
> index f7d06050f7..ef40ae512a 100644
> --- a/util/osdep.c
> +++ b/util/osdep.c
> @@ -484,7 +484,7 @@ void fips_set_state(bool requested)
> #endif /* __linux__ */
>
> #ifdef _FIPS_DEBUG
> - fprintf(stderr, "FIPS mode %s (requested %s)\n",
> + error_report("FIPS mode %s (requested %s)",
> (fips_enabled ? "enabled" : "disabled"),
> (requested ? "enabled" : "disabled"));
> #endif
> @@ -511,7 +511,7 @@ int socket_init(void)
> ret = WSAStartup(MAKEWORD(2, 2), &Data);
> if (ret != 0) {
> err = WSAGetLastError();
> - fprintf(stderr, "WSAStartup: %d\n", err);
> + error_report("WSAStartup: %d", err);
> return -1;
> }
> atexit(socket_cleanup);
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 897e8f3ba6..4977594a43 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -35,6 +35,7 @@
> #include "sysemu/sysemu.h"
> #include "trace.h"
> #include "qapi/error.h"
> +#include "qemu/error-report.h"
> #include "qemu/sockets.h"
> #include "qemu/thread.h"
> #include <libgen.h>
> @@ -170,7 +171,7 @@ fail_close:
> void *qemu_oom_check(void *ptr)
> {
> if (ptr == NULL) {
> - fprintf(stderr, "Failed to allocate memory: %s\n", strerror(errno));
> + error_report("Failed to allocate memory: %s", strerror(errno));
> abort();
> }
> return ptr;
> diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> index e9b14ab178..84b937865a 100644
> --- a/util/oslib-win32.c
> +++ b/util/oslib-win32.c
> @@ -39,6 +39,7 @@
> #include "trace.h"
> #include "qemu/sockets.h"
> #include "qemu/cutils.h"
> +#include "qemu/error-report.h"
>
> /* this must come after including "trace.h" */
> #include <shlobj.h>
> @@ -46,7 +47,7 @@
> void *qemu_oom_check(void *ptr)
> {
> if (ptr == NULL) {
> - fprintf(stderr, "Failed to allocate memory: %lu\n", GetLastError());
> + error_report("Failed to allocate memory: %lu", GetLastError());
> abort();
> }
> return ptr;
> diff --git a/util/qemu-coroutine.c b/util/qemu-coroutine.c
> index c3caa6c770..62d1dd09df 100644
> --- a/util/qemu-coroutine.c
> +++ b/util/qemu-coroutine.c
> @@ -14,6 +14,7 @@
>
> #include "qemu/osdep.h"
> #include "trace.h"
> +#include "qemu/error-report.h"
> #include "qemu/thread.h"
> #include "qemu/atomic.h"
> #include "qemu/coroutine.h"
> @@ -125,14 +126,13 @@ void qemu_aio_coroutine_enter(AioContext *ctx,
> Coroutine *co)
> * cause us to enter it twice, potentially even after the coroutine
> has
> * been deleted */
> if (scheduled) {
> - fprintf(stderr,
> - "%s: Co-routine was already scheduled in '%s'\n",
> - __func__, scheduled);
> + error_report("%s: Co-routine was already scheduled in '%s'",
> + __func__, scheduled);
> abort();
> }
>
> if (to->caller) {
> - fprintf(stderr, "Co-routine re-entered recursively\n");
> + error_report("Co-routine re-entered recursively");
> abort();
> }
>
> @@ -185,7 +185,7 @@ void coroutine_fn qemu_coroutine_yield(void)
> trace_qemu_coroutine_yield(self, to);
>
> if (!to) {
> - fprintf(stderr, "Co-routine is yielding to no one\n");
> + error_report("Co-routine is yielding to no one");
> abort();
> }
>
> diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
> index 838980aaa5..b4d8de376c 100644
> --- a/util/qemu-thread-posix.c
> +++ b/util/qemu-thread-posix.c
> @@ -14,6 +14,7 @@
> #include "qemu/thread.h"
> #include "qemu/atomic.h"
> #include "qemu/notify.h"
> +#include "qemu/error-report.h"
> #include "qemu-thread-common.h"
>
> static bool name_threads;
> @@ -25,14 +26,14 @@ void qemu_thread_naming(bool enable)
> #ifndef CONFIG_THREAD_SETNAME_BYTHREAD
> /* This is a debugging option, not fatal */
> if (enable) {
> - fprintf(stderr, "qemu: thread naming not supported on this host\n");
> + error_report("qemu: thread naming not supported on this host");
This isn't an error. It's in response to -name debug-threads=on, and
tells the user debug-threads=on is being ignored. Let's use
warn_report().
Drop the "qemu: ", please; error_report() & friends take care of that.
More of the same below.
> }
> #endif
> }
>
> static void error_exit(int err, const char *msg)
> {
> - fprintf(stderr, "qemu: %s: %s\n", msg, strerror(err));
> + error_report("qemu: %s: %s", msg, strerror(err));
> abort();
> }
>
> diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c
> index 56a83333da..9bed338d7e 100644
> --- a/util/qemu-thread-win32.c
> +++ b/util/qemu-thread-win32.c
> @@ -15,6 +15,7 @@
> #include "qemu-common.h"
> #include "qemu/thread.h"
> #include "qemu/notify.h"
> +#include "qemu/error-report.h"
> #include "qemu-thread-common.h"
> #include <process.h>
>
> @@ -25,7 +26,7 @@ void qemu_thread_naming(bool enable)
> /* But note we don't actually name them on Windows yet */
> name_threads = enable;
>
> - fprintf(stderr, "qemu: thread naming not supported on this host\n");
> + error_report("qemu: thread naming not supported on this host");
Likewise.
> }
>
> static void error_exit(int err, const char *msg)
> @@ -34,7 +35,7 @@ static void error_exit(int err, const char *msg)
>
> FormatMessage(FORMAT_MESSAGE_FROM_SYSTEM |
> FORMAT_MESSAGE_ALLOCATE_BUFFER,
> NULL, err, 0, (LPTSTR)&pstr, 2, NULL);
> - fprintf(stderr, "qemu: %s: %s\n", msg, pstr);
> + error_report("qemu: %s: %s", msg, pstr);
> LocalFree(pstr);
> abort();
> }
> diff --git a/util/qemu-timer-common.c b/util/qemu-timer-common.c
> index baf3317f74..527944da1c 100644
> --- a/util/qemu-timer-common.c
> +++ b/util/qemu-timer-common.c
> @@ -23,6 +23,7 @@
> */
> #include "qemu/osdep.h"
> #include "qemu/timer.h"
> +#include "qemu/error-report.h"
>
> /***********************************************************/
> /* real time host monotonic timer */
> @@ -37,7 +38,7 @@ static void __attribute__((constructor))
> init_get_clock(void)
> int ret;
> ret = QueryPerformanceFrequency(&freq);
> if (ret == 0) {
> - fprintf(stderr, "Could not calibrate ticks\n");
> + error_report("Could not calibrate ticks");
> exit(1);
> }
> clock_freq = freq.QuadPart;
[PATCH v2 3/6] util/oslib-win32: Improve error report by calling error_setg_win32(), Philippe Mathieu-Daudé, 2020/02/27
[PATCH v2 5/6] qga: Fix a memory leak, Philippe Mathieu-Daudé, 2020/02/27
[PATCH v2 4/6] util/osdep: Improve error report by calling error_setg_win32(), Philippe Mathieu-Daudé, 2020/02/27
[PATCH v2 6/6] qga: Improve error report by calling error_setg_win32(), Philippe Mathieu-Daudé, 2020/02/27