On Wed, Oct 21, 2020 at 11:19 AM Markus Armbruster <arm...@redhat.com>
wrote:
> The Error ** argument must be NULL, &error_abort, &error_fatal, or a
> pointer to a variable containing NULL. Passing an argument of the
> latter kind twice without clearing it in between is wrong: if the
> first call sets an error, it no longer points to NULL for the second
> call.
>
> qmp_guest_get_devices() is wrong that way: it calls error_setg() in a
> loop.
>
> If no iteration fails, the function returns a value and sets no error.
> Okay.
>
> If exactly one iteration fails, the function returns a value and sets
> an error. Wrong.
>
> If multiple iterations fail, the function trips error_setv()'s
> assertion.
>
> Fix it to return immediately on error.
>
> Perhaps the failure to convert the driver version to UTF-8 should not
> be an error. We could simply not report the botched version string
> instead.
>
> Drop a superfluous continue while there.
>
> Signed-off-by: Markus Armbruster <arm...@redhat.com>
>
Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> ---
> qga/commands-win32.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index b01616a992..1efe3ba076 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -2385,7 +2385,7 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error
> **errp)
> device->driver_name = g_utf16_to_utf8(name, -1, NULL, NULL, NULL);
> if (device->driver_name == NULL) {
> error_setg(errp, "conversion to utf8 failed (driver name)");
> - continue;
> + return NULL;
> }
> slog("querying device: %s", device->driver_name);
> hw_ids = ga_get_hardware_ids(dev_info_data.DevInst);
> @@ -2428,7 +2428,7 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error
> **errp)
> NULL, NULL);
> if (device->driver_version == NULL) {
> error_setg(errp, "conversion to utf8 failed (driver
> version)");
> - continue;
> + return NULL;
> }
> device->has_driver_version = true;
>
> @@ -2452,7 +2452,6 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error
> **errp)
> cur_item->next = item;
> cur_item = item;
> }
> - continue;
> }
>
> if (dev_info != INVALID_HANDLE_VALUE) {
> --
> 2.26.2
>
>
>
--
Marc-André Lureau
