On Wed, Mar 20, 2024 at 04:36:40PM +0800, Tao Su wrote: > monitor_puts() doesn't check the monitor pointer, but do_inject_x86_mce() > may have a parameter with NULL monitor pointer. Revert monitor_puts() in > do_inject_x86_mce() to fix, then the fact that we send the same message to > monitor and log is again more obvious.
Yikes, why do we have such a horrible trap-door in our monitor output APIs. Isn't the right fix here to make 'monitor_puts' check for NULL & be a no-op, in the same way 'monitor_printf' does, so the APIs have consistent behaviour. > > Fixes: bf0c50d4aa85 (monitor: expose monitor_puts to rest of code) > Reviwed-by: Xiaoyao Li <xiaoyao...@intel.com> > Reviewed-by: Markus Armbruster <arm...@redhat.com> > Signed-off-by: Tao Su <tao1...@linux.intel.com> > --- > v1 -> v2: > - Instead revert the broken part of commit bf0c50d4aa85 > - Add Markus's Reviewed-by > > v1: > - https://lore.kernel.org/all/20240320052118.520378-1-tao1...@linux.intel.com/ > --- > target/i386/helper.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/i386/helper.c b/target/i386/helper.c > index 2070dd0dda..23ccb23a5b 100644 > --- a/target/i386/helper.c > +++ b/target/i386/helper.c > @@ -430,7 +430,7 @@ static void do_inject_x86_mce(CPUState *cs, > run_on_cpu_data data) > if (need_reset) { > emit_guest_memory_failure(MEMORY_FAILURE_ACTION_RESET, ar, > recursive); > - monitor_puts(params->mon, msg); > + monitor_printf(params->mon, "%s", msg); > qemu_log_mask(CPU_LOG_RESET, "%s\n", msg); > qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET); > return; > > base-commit: c62d54d0a8067ffb3d5b909276f7296d7df33fa7 > -- > 2.34.1 > > With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
