[PATCH 1/1] The virJSONValueObjectGetString function can return NULL, there are no checks

Peter Krempa pkrempa at redhat.com
Tue Oct 10 15:24:32 UTC 2023


On Tue, Oct 10, 2023 at 17:55:26 +0300, Sergey Mironov wrote:
> The devAlias check is performed in the "if (!devAlias && !devid) {", so the situation is not handled when devAlias is NULL, and devid is not NULL
> 
> Signed-off-by: Sergey Mironov <mironov at fintech.ru>
> ---
>  src/qemu/qemu_monitor_json.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 5b9edadcf7..80b41d77d3 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -933,6 +933,11 @@ qemuMonitorJSONHandleTrayChange(qemuMonitor *mon,
>      bool trayOpened;
>      int reason;
>  
> +    if (!devAlias) {
> +        VIR_WARN("missing device in tray change event");
> +        return;
> +    }

This is not correct and based on a wrong assumption. Currently the code
always gets a 'devAlias' set but it *may* be an empty string.

If it is an empty string it's cleared first before continuing.

Then if both the alias and 'id' is NULL the function terminates.

Making the code more robust in case when qemu stops sending a devAlias
is a good thing though, but the fix propsed here would break the
function as it would end early even if 'devid' is non-NULL. The
code afterwards is okay with devAlias being NULL.

Thus the correct fix is for the condition to read:

  if (devAlias && *devAlias == '\0')
      devAlias = NULL;

and leave the other logic as-is.


More information about the libvir-list mailing list