Daniel P. Berrangé <[email protected]> writes:
> One codepath that could return NULL failed to populate the errp
> object.
>
> Reported-by: Markus Armbruster <[email protected]>
> Signed-off-by: Daniel P. Berrangé <[email protected]>
> ---
> 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 <[email protected]>
