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

[Peter Krempa <pkrempa@xxxxxxxxxx>]

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@xxxxxxxxxx>
> ---
>  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.