Daniel P. Berrangé <berrange@redhat.com> writes:
One codepath that could return NULL failed to populate the errp object.
Reported-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- util/log.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/util/log.c b/util/log.c index 1644e6814b..1c9c7adb2d 100644 --- a/util/log.c +++ b/util/log.c @@ -118,6 +118,7 @@ static FILE *qemu_log_trylock_with_err(Error **errp) static FILE *qemu_log_trylock_with_err(Error **errp) { FILE *logfile;
logfile = thread_file; if (!logfile) { Slow path: no log file cached for this thread. if (log_per_thread) { We're logging to per-thread log files. @global_filename is a file name pattern; derive this thread's log file from it. g_autofree char *filename = g_strdup_printf(global_filename, log_thread_id()); logfile = fopen(filename, "w"); if (!logfile) { error_setg_errno(errp, errno, "Error opening logfile %s for thread %d", filename, log_thread_id()); return NULL; } thread_file = logfile; qemu_log_thread_cleanup_notifier.notify = qemu_log_thread_cleanup; qemu_thread_atexit_add(&qemu_log_thread_cleanup_notifier); } else { Logging to a single log file. Fetch it from @global_file. rcu_read_lock(); /* * FIXME: typeof_strip_qual, as used by qatomic_rcu_read, * does not work with pointers to undefined structures, * such as we have with struct _IO_FILE and musl libc. * Since all we want is a read of a pointer, cast to void**, * which does work with typeof_strip_qual. */
logfile = qatomic_rcu_read((void **)&global_file); if (!logfile) {
@global_file was null when we fetched it. It can be null when @daemonized and the user didn't set a log file.
rcu_read_unlock(); + error_setg(errp, "Global log file output is not open");
Impact? The only callers passing non-null @errp is qemu_set_log_internal(), and it can call only when @log_per_thread. We can't reach the new error then. So this is just a latent bug. Worth mentioning in the commit message?
return NULL; }
We do not set thread_file. We'll take the slow path again. Harmless enough, but its a bit at odds with how the code is structured here. Observation, not a demand.
}
} qemu_flockfile(logfile); return logfile; } Reviewed-by: Markus Armbruster <armbru@redhat.com>