[PATCH 0/1] The virJSONValueObjectGetString function can return NULL, there are no checks
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 Sergey Mironov (1): The virJSONValueObjectGetString function can return NULL, there are no checks src/qemu/qemu_monitor_json.c | 5 +++++ 1 file changed, 5 insertions(+) -- 2.31.1
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@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; + } + /* drive alias is always reported but empty for -blockdev */ if (*devAlias == '\0') devAlias = NULL; -- 2.31.1
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@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.
participants (2)
-
Peter Krempa -
Sergey Mironov