* [PATCH v2 1/6] gqa-win: get_pci_info: Clean dev_info if handle is valid
2021-12-14 12:41 [PATCH v2 0/6] gqa-win: get_pci_info: Fix memory leak Kostiantyn Kostiuk
@ 2021-12-14 12:41 ` Kostiantyn Kostiuk
2021-12-14 13:26 ` Marc-André Lureau
2021-12-14 12:41 ` [PATCH v2 2/6] gqa-win: get_pci_info: Use common 'end' label Kostiantyn Kostiuk
` (4 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Kostiantyn Kostiuk @ 2021-12-14 12:41 UTC (permalink / raw)
To: qemu-devel, Michael Roth, Marc-André Lureau,
Philippe Mathieu-Daudé
Signed-off-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
Signed-off-by: Kostiantyn Kostiuk <konstantin@daynix.com>
---
qga/commands-win32.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 4e84afd83b..3dd74fe225 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -514,7 +514,7 @@ DEFINE_GUID(GUID_DEVINTERFACE_STORAGEPORT,
static GuestPCIAddress *get_pci_info(int number, Error **errp)
{
- HDEVINFO dev_info;
+ HDEVINFO dev_info = INVALID_HANDLE_VALUE;
SP_DEVINFO_DATA dev_info_data;
SP_DEVICE_INTERFACE_DATA dev_iface_data;
HANDLE dev_file;
@@ -749,7 +749,9 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
}
free_dev_info:
- SetupDiDestroyDeviceInfoList(dev_info);
+ if (dev_info != INVALID_HANDLE_VALUE) {
+ SetupDiDestroyDeviceInfoList(dev_info);
+ }
out:
return pci;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 1/6] gqa-win: get_pci_info: Clean dev_info if handle is valid
2021-12-14 12:41 ` [PATCH v2 1/6] gqa-win: get_pci_info: Clean dev_info if handle is valid Kostiantyn Kostiuk
@ 2021-12-14 13:26 ` Marc-André Lureau
0 siblings, 0 replies; 13+ messages in thread
From: Marc-André Lureau @ 2021-12-14 13:26 UTC (permalink / raw)
To: Kostiantyn Kostiuk; +Cc: Michael Roth, Philippe Mathieu-Daudé, qemu-devel
On Tue, Dec 14, 2021 at 4:41 PM Kostiantyn Kostiuk
<konstantin@daynix.com> wrote:
>
> Signed-off-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
> Signed-off-by: Kostiantyn Kostiuk <konstantin@daynix.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> qga/commands-win32.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index 4e84afd83b..3dd74fe225 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -514,7 +514,7 @@ DEFINE_GUID(GUID_DEVINTERFACE_STORAGEPORT,
>
> static GuestPCIAddress *get_pci_info(int number, Error **errp)
> {
> - HDEVINFO dev_info;
> + HDEVINFO dev_info = INVALID_HANDLE_VALUE;
> SP_DEVINFO_DATA dev_info_data;
> SP_DEVICE_INTERFACE_DATA dev_iface_data;
> HANDLE dev_file;
> @@ -749,7 +749,9 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
> }
>
> free_dev_info:
> - SetupDiDestroyDeviceInfoList(dev_info);
> + if (dev_info != INVALID_HANDLE_VALUE) {
> + SetupDiDestroyDeviceInfoList(dev_info);
> + }
> out:
> return pci;
> }
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 2/6] gqa-win: get_pci_info: Use common 'end' label
2021-12-14 12:41 [PATCH v2 0/6] gqa-win: get_pci_info: Fix memory leak Kostiantyn Kostiuk
2021-12-14 12:41 ` [PATCH v2 1/6] gqa-win: get_pci_info: Clean dev_info if handle is valid Kostiantyn Kostiuk
@ 2021-12-14 12:41 ` Kostiantyn Kostiuk
2021-12-14 13:26 ` Marc-André Lureau
2021-12-14 12:41 ` [PATCH v2 3/6] gqa-win: get_pci_info: Free parent_dev_info properly Kostiantyn Kostiuk
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Kostiantyn Kostiuk @ 2021-12-14 12:41 UTC (permalink / raw)
To: qemu-devel, Michael Roth, Marc-André Lureau,
Philippe Mathieu-Daudé
To prevent memory leaks, always try to free initialized variables.
Signed-off-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
Signed-off-by: Kostiantyn Kostiuk <konstantin@daynix.com>
---
qga/commands-win32.c | 25 ++++++++++++-------------
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 3dd74fe225..93c5375132 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -532,7 +532,7 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
DIGCF_PRESENT | DIGCF_DEVICEINTERFACE);
if (dev_info == INVALID_HANDLE_VALUE) {
error_setg_win32(errp, GetLastError(), "failed to get devices tree");
- goto out;
+ goto end;
}
g_debug("enumerating devices");
@@ -562,7 +562,7 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
} else {
error_setg_win32(errp, GetLastError(),
"failed to get device interfaces");
- goto free_dev_info;
+ goto end;
}
}
@@ -576,7 +576,7 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
CloseHandle(dev_file);
error_setg_win32(errp, GetLastError(),
"failed to get device slot number");
- goto free_dev_info;
+ goto end;
}
CloseHandle(dev_file);
@@ -586,7 +586,7 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
} else {
error_setg_win32(errp, GetLastError(),
"failed to get device interfaces");
- goto free_dev_info;
+ goto end;
}
g_debug("found device slot %d. Getting storage controller", number);
@@ -603,7 +603,7 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
} else {
error_setg_win32(errp, GetLastError(),
"failed to get device instance ID");
- goto out;
+ goto end;
}
}
@@ -617,14 +617,14 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
g_error("CM_Locate_DevInst failed with code %lx", cr);
error_setg_win32(errp, GetLastError(),
"failed to get device instance");
- goto out;
+ goto end;
}
cr = CM_Get_Parent(&parent_dev_inst, dev_inst, 0);
if (cr != CR_SUCCESS) {
g_error("CM_Get_Parent failed with code %lx", cr);
error_setg_win32(errp, GetLastError(),
"failed to get parent device instance");
- goto out;
+ goto end;
}
cr = CM_Get_Device_ID_Size(&dev_id_size, parent_dev_inst, 0);
@@ -632,7 +632,7 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
g_error("CM_Get_Device_ID_Size failed with code %lx", cr);
error_setg_win32(errp, GetLastError(),
"failed to get parent device ID length");
- goto out;
+ goto end;
}
++dev_id_size;
@@ -647,7 +647,7 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
g_error("CM_Get_Device_ID failed with code %lx", cr);
error_setg_win32(errp, GetLastError(),
"failed to get parent device ID");
- goto out;
+ goto end;
}
}
@@ -661,14 +661,14 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
if (parent_dev_info == INVALID_HANDLE_VALUE) {
error_setg_win32(errp, GetLastError(),
"failed to get parent device");
- goto out;
+ goto end;
}
parent_dev_info_data.cbSize = sizeof(SP_DEVINFO_DATA);
if (!SetupDiEnumDeviceInfo(parent_dev_info, 0, &parent_dev_info_data)) {
error_setg_win32(errp, GetLastError(),
"failed to get parent device data");
- goto out;
+ goto end;
}
for (j = 0;
@@ -748,11 +748,10 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
break;
}
-free_dev_info:
+end:
if (dev_info != INVALID_HANDLE_VALUE) {
SetupDiDestroyDeviceInfoList(dev_info);
}
-out:
return pci;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 2/6] gqa-win: get_pci_info: Use common 'end' label
2021-12-14 12:41 ` [PATCH v2 2/6] gqa-win: get_pci_info: Use common 'end' label Kostiantyn Kostiuk
@ 2021-12-14 13:26 ` Marc-André Lureau
0 siblings, 0 replies; 13+ messages in thread
From: Marc-André Lureau @ 2021-12-14 13:26 UTC (permalink / raw)
To: Kostiantyn Kostiuk; +Cc: Michael Roth, Philippe Mathieu-Daudé, qemu-devel
On Tue, Dec 14, 2021 at 4:41 PM Kostiantyn Kostiuk
<konstantin@daynix.com> wrote:
>
> To prevent memory leaks, always try to free initialized variables.
>
> Signed-off-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
> Signed-off-by: Kostiantyn Kostiuk <konstantin@daynix.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> qga/commands-win32.c | 25 ++++++++++++-------------
> 1 file changed, 12 insertions(+), 13 deletions(-)
>
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index 3dd74fe225..93c5375132 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -532,7 +532,7 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
> DIGCF_PRESENT | DIGCF_DEVICEINTERFACE);
> if (dev_info == INVALID_HANDLE_VALUE) {
> error_setg_win32(errp, GetLastError(), "failed to get devices tree");
> - goto out;
> + goto end;
> }
>
> g_debug("enumerating devices");
> @@ -562,7 +562,7 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
> } else {
> error_setg_win32(errp, GetLastError(),
> "failed to get device interfaces");
> - goto free_dev_info;
> + goto end;
> }
> }
>
> @@ -576,7 +576,7 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
> CloseHandle(dev_file);
> error_setg_win32(errp, GetLastError(),
> "failed to get device slot number");
> - goto free_dev_info;
> + goto end;
> }
>
> CloseHandle(dev_file);
> @@ -586,7 +586,7 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
> } else {
> error_setg_win32(errp, GetLastError(),
> "failed to get device interfaces");
> - goto free_dev_info;
> + goto end;
> }
>
> g_debug("found device slot %d. Getting storage controller", number);
> @@ -603,7 +603,7 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
> } else {
> error_setg_win32(errp, GetLastError(),
> "failed to get device instance ID");
> - goto out;
> + goto end;
> }
> }
>
> @@ -617,14 +617,14 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
> g_error("CM_Locate_DevInst failed with code %lx", cr);
> error_setg_win32(errp, GetLastError(),
> "failed to get device instance");
> - goto out;
> + goto end;
> }
> cr = CM_Get_Parent(&parent_dev_inst, dev_inst, 0);
> if (cr != CR_SUCCESS) {
> g_error("CM_Get_Parent failed with code %lx", cr);
> error_setg_win32(errp, GetLastError(),
> "failed to get parent device instance");
> - goto out;
> + goto end;
> }
>
> cr = CM_Get_Device_ID_Size(&dev_id_size, parent_dev_inst, 0);
> @@ -632,7 +632,7 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
> g_error("CM_Get_Device_ID_Size failed with code %lx", cr);
> error_setg_win32(errp, GetLastError(),
> "failed to get parent device ID length");
> - goto out;
> + goto end;
> }
>
> ++dev_id_size;
> @@ -647,7 +647,7 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
> g_error("CM_Get_Device_ID failed with code %lx", cr);
> error_setg_win32(errp, GetLastError(),
> "failed to get parent device ID");
> - goto out;
> + goto end;
> }
> }
>
> @@ -661,14 +661,14 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
> if (parent_dev_info == INVALID_HANDLE_VALUE) {
> error_setg_win32(errp, GetLastError(),
> "failed to get parent device");
> - goto out;
> + goto end;
> }
>
> parent_dev_info_data.cbSize = sizeof(SP_DEVINFO_DATA);
> if (!SetupDiEnumDeviceInfo(parent_dev_info, 0, &parent_dev_info_data)) {
> error_setg_win32(errp, GetLastError(),
> "failed to get parent device data");
> - goto out;
> + goto end;
> }
>
> for (j = 0;
> @@ -748,11 +748,10 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
> break;
> }
>
> -free_dev_info:
> +end:
> if (dev_info != INVALID_HANDLE_VALUE) {
> SetupDiDestroyDeviceInfoList(dev_info);
> }
> -out:
> return pci;
> }
>
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 3/6] gqa-win: get_pci_info: Free parent_dev_info properly
2021-12-14 12:41 [PATCH v2 0/6] gqa-win: get_pci_info: Fix memory leak Kostiantyn Kostiuk
2021-12-14 12:41 ` [PATCH v2 1/6] gqa-win: get_pci_info: Clean dev_info if handle is valid Kostiantyn Kostiuk
2021-12-14 12:41 ` [PATCH v2 2/6] gqa-win: get_pci_info: Use common 'end' label Kostiantyn Kostiuk
@ 2021-12-14 12:41 ` Kostiantyn Kostiuk
2021-12-14 13:26 ` Marc-André Lureau
2021-12-14 12:41 ` [PATCH v2 4/6] gqa-win: get_pci_info: Split logic to separate functions Kostiantyn Kostiuk
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Kostiantyn Kostiuk @ 2021-12-14 12:41 UTC (permalink / raw)
To: qemu-devel, Michael Roth, Marc-André Lureau,
Philippe Mathieu-Daudé
In case when the function fails to get parent device data,
the parent_dev_info variable will be initialized, but not freed.
Signed-off-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
Signed-off-by: Kostiantyn Kostiuk <konstantin@daynix.com>
---
qga/commands-win32.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 93c5375132..f6de9e2676 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -515,6 +515,8 @@ DEFINE_GUID(GUID_DEVINTERFACE_STORAGEPORT,
static GuestPCIAddress *get_pci_info(int number, Error **errp)
{
HDEVINFO dev_info = INVALID_HANDLE_VALUE;
+ HDEVINFO parent_dev_info = INVALID_HANDLE_VALUE;
+
SP_DEVINFO_DATA dev_info_data;
SP_DEVICE_INTERFACE_DATA dev_iface_data;
HANDLE dev_file;
@@ -542,7 +544,6 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
PSP_DEVICE_INTERFACE_DETAIL_DATA pdev_iface_detail_data = NULL;
STORAGE_DEVICE_NUMBER sdn;
char *parent_dev_id = NULL;
- HDEVINFO parent_dev_info;
SP_DEVINFO_DATA parent_dev_info_data;
DWORD j;
DWORD size = 0;
@@ -744,11 +745,13 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
break;
}
}
- SetupDiDestroyDeviceInfoList(parent_dev_info);
break;
}
end:
+ if (parent_dev_info != INVALID_HANDLE_VALUE) {
+ SetupDiDestroyDeviceInfoList(parent_dev_info);
+ }
if (dev_info != INVALID_HANDLE_VALUE) {
SetupDiDestroyDeviceInfoList(dev_info);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 3/6] gqa-win: get_pci_info: Free parent_dev_info properly
2021-12-14 12:41 ` [PATCH v2 3/6] gqa-win: get_pci_info: Free parent_dev_info properly Kostiantyn Kostiuk
@ 2021-12-14 13:26 ` Marc-André Lureau
0 siblings, 0 replies; 13+ messages in thread
From: Marc-André Lureau @ 2021-12-14 13:26 UTC (permalink / raw)
To: Kostiantyn Kostiuk; +Cc: Michael Roth, Philippe Mathieu-Daudé, qemu-devel
On Tue, Dec 14, 2021 at 4:41 PM Kostiantyn Kostiuk
<konstantin@daynix.com> wrote:
>
> In case when the function fails to get parent device data,
> the parent_dev_info variable will be initialized, but not freed.
>
> Signed-off-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
> Signed-off-by: Kostiantyn Kostiuk <konstantin@daynix.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> qga/commands-win32.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index 93c5375132..f6de9e2676 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -515,6 +515,8 @@ DEFINE_GUID(GUID_DEVINTERFACE_STORAGEPORT,
> static GuestPCIAddress *get_pci_info(int number, Error **errp)
> {
> HDEVINFO dev_info = INVALID_HANDLE_VALUE;
> + HDEVINFO parent_dev_info = INVALID_HANDLE_VALUE;
> +
> SP_DEVINFO_DATA dev_info_data;
> SP_DEVICE_INTERFACE_DATA dev_iface_data;
> HANDLE dev_file;
> @@ -542,7 +544,6 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
> PSP_DEVICE_INTERFACE_DETAIL_DATA pdev_iface_detail_data = NULL;
> STORAGE_DEVICE_NUMBER sdn;
> char *parent_dev_id = NULL;
> - HDEVINFO parent_dev_info;
> SP_DEVINFO_DATA parent_dev_info_data;
> DWORD j;
> DWORD size = 0;
> @@ -744,11 +745,13 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
> break;
> }
> }
> - SetupDiDestroyDeviceInfoList(parent_dev_info);
> break;
> }
>
> end:
> + if (parent_dev_info != INVALID_HANDLE_VALUE) {
> + SetupDiDestroyDeviceInfoList(parent_dev_info);
> + }
> if (dev_info != INVALID_HANDLE_VALUE) {
> SetupDiDestroyDeviceInfoList(dev_info);
> }
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 4/6] gqa-win: get_pci_info: Split logic to separate functions
2021-12-14 12:41 [PATCH v2 0/6] gqa-win: get_pci_info: Fix memory leak Kostiantyn Kostiuk
` (2 preceding siblings ...)
2021-12-14 12:41 ` [PATCH v2 3/6] gqa-win: get_pci_info: Free parent_dev_info properly Kostiantyn Kostiuk
@ 2021-12-14 12:41 ` Kostiantyn Kostiuk
2021-12-14 13:27 ` Marc-André Lureau
2021-12-14 12:41 ` [PATCH v2 5/6] gqa-win: get_pci_info: Add g_autofree for few variables Kostiantyn Kostiuk
2021-12-14 12:41 ` [PATCH v2 6/6] gqa-win: get_pci_info: Replace 'while' with 2 calls of the function Kostiantyn Kostiuk
5 siblings, 1 reply; 13+ messages in thread
From: Kostiantyn Kostiuk @ 2021-12-14 12:41 UTC (permalink / raw)
To: qemu-devel, Michael Roth, Marc-André Lureau,
Philippe Mathieu-Daudé
Signed-off-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
Signed-off-by: Kostiantyn Kostiuk <konstantin@daynix.com>
---
qga/commands-win32.c | 161 +++++++++++++++++++++++--------------------
1 file changed, 87 insertions(+), 74 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index f6de9e2676..8588fa8633 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -512,6 +512,92 @@ DEFINE_GUID(GUID_DEVINTERFACE_STORAGEPORT,
0x2accfe60L, 0xc130, 0x11d2, 0xb0, 0x82,
0x00, 0xa0, 0xc9, 0x1e, 0xfb, 0x8b);
+static void get_pci_address_for_device(GuestPCIAddress *pci,
+ HDEVINFO dev_info)
+{
+ SP_DEVINFO_DATA dev_info_data;
+ DWORD j;
+ DWORD size;
+ bool partial_pci = false;
+
+ dev_info_data.cbSize = sizeof(SP_DEVINFO_DATA);
+
+ for (j = 0;
+ SetupDiEnumDeviceInfo(dev_info, j, &dev_info_data);
+ j++) {
+ DWORD addr, bus, ui_slot, type;
+ int func, slot;
+ size = sizeof(DWORD);
+
+ /*
+ * There is no need to allocate buffer in the next functions. The
+ * size is known and ULONG according to
+ * https://msdn.microsoft.com/en-us/library/windows/hardware/ff543095(v=vs.85).aspx
+ */
+ if (!SetupDiGetDeviceRegistryProperty(
+ dev_info, &dev_info_data, SPDRP_BUSNUMBER,
+ &type, (PBYTE)&bus, size, NULL)) {
+ debug_error("failed to get PCI bus");
+ bus = -1;
+ partial_pci = true;
+ }
+
+ /*
+ * The function retrieves the device's address. This value will be
+ * transformed into device function and number
+ */
+ if (!SetupDiGetDeviceRegistryProperty(
+ dev_info, &dev_info_data, SPDRP_ADDRESS,
+ &type, (PBYTE)&addr, size, NULL)) {
+ debug_error("failed to get PCI address");
+ addr = -1;
+ partial_pci = true;
+ }
+
+ /*
+ * This call returns UINumber of DEVICE_CAPABILITIES structure.
+ * This number is typically a user-perceived slot number.
+ */
+ if (!SetupDiGetDeviceRegistryProperty(
+ dev_info, &dev_info_data, SPDRP_UI_NUMBER,
+ &type, (PBYTE)&ui_slot, size, NULL)) {
+ debug_error("failed to get PCI slot");
+ ui_slot = -1;
+ partial_pci = true;
+ }
+
+ /*
+ * SetupApi gives us the same information as driver with
+ * IoGetDeviceProperty. According to Microsoft:
+ *
+ * FunctionNumber = (USHORT)((propertyAddress) & 0x0000FFFF)
+ * DeviceNumber = (USHORT)(((propertyAddress) >> 16) & 0x0000FFFF)
+ * SPDRP_ADDRESS is propertyAddress, so we do the same.
+ *
+ * https://docs.microsoft.com/en-us/windows/desktop/api/setupapi/nf-setupapi-setupdigetdeviceregistrypropertya
+ */
+ if (partial_pci) {
+ pci->domain = -1;
+ pci->slot = -1;
+ pci->function = -1;
+ pci->bus = -1;
+ continue;
+ } else {
+ func = ((int)addr == -1) ? -1 : addr & 0x0000FFFF;
+ slot = ((int)addr == -1) ? -1 : (addr >> 16) & 0x0000FFFF;
+ if ((int)ui_slot != slot) {
+ g_debug("mismatch with reported slot values: %d vs %d",
+ (int)ui_slot, slot);
+ }
+ pci->domain = 0;
+ pci->slot = (int)ui_slot;
+ pci->function = func;
+ pci->bus = (int)bus;
+ return;
+ }
+ }
+}
+
static GuestPCIAddress *get_pci_info(int number, Error **errp)
{
HDEVINFO dev_info = INVALID_HANDLE_VALUE;
@@ -522,7 +608,6 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
HANDLE dev_file;
int i;
GuestPCIAddress *pci = NULL;
- bool partial_pci = false;
pci = g_malloc0(sizeof(*pci));
pci->domain = -1;
@@ -545,7 +630,6 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
STORAGE_DEVICE_NUMBER sdn;
char *parent_dev_id = NULL;
SP_DEVINFO_DATA parent_dev_info_data;
- DWORD j;
DWORD size = 0;
g_debug("getting device path");
@@ -672,79 +756,8 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
goto end;
}
- for (j = 0;
- SetupDiEnumDeviceInfo(parent_dev_info, j, &parent_dev_info_data);
- j++) {
- DWORD addr, bus, ui_slot, type;
- int func, slot;
+ get_pci_address_for_device(pci, parent_dev_info);
- /*
- * There is no need to allocate buffer in the next functions. The
- * size is known and ULONG according to
- * https://msdn.microsoft.com/en-us/library/windows/hardware/ff543095(v=vs.85).aspx
- */
- if (!SetupDiGetDeviceRegistryProperty(
- parent_dev_info, &parent_dev_info_data, SPDRP_BUSNUMBER,
- &type, (PBYTE)&bus, size, NULL)) {
- debug_error("failed to get PCI bus");
- bus = -1;
- partial_pci = true;
- }
-
- /*
- * The function retrieves the device's address. This value will be
- * transformed into device function and number
- */
- if (!SetupDiGetDeviceRegistryProperty(
- parent_dev_info, &parent_dev_info_data, SPDRP_ADDRESS,
- &type, (PBYTE)&addr, size, NULL)) {
- debug_error("failed to get PCI address");
- addr = -1;
- partial_pci = true;
- }
-
- /*
- * This call returns UINumber of DEVICE_CAPABILITIES structure.
- * This number is typically a user-perceived slot number.
- */
- if (!SetupDiGetDeviceRegistryProperty(
- parent_dev_info, &parent_dev_info_data, SPDRP_UI_NUMBER,
- &type, (PBYTE)&ui_slot, size, NULL)) {
- debug_error("failed to get PCI slot");
- ui_slot = -1;
- partial_pci = true;
- }
-
- /*
- * SetupApi gives us the same information as driver with
- * IoGetDeviceProperty. According to Microsoft:
- *
- * FunctionNumber = (USHORT)((propertyAddress) & 0x0000FFFF)
- * DeviceNumber = (USHORT)(((propertyAddress) >> 16) & 0x0000FFFF)
- * SPDRP_ADDRESS is propertyAddress, so we do the same.
- *
- * https://docs.microsoft.com/en-us/windows/desktop/api/setupapi/nf-setupapi-setupdigetdeviceregistrypropertya
- */
- if (partial_pci) {
- pci->domain = -1;
- pci->slot = -1;
- pci->function = -1;
- pci->bus = -1;
- continue;
- } else {
- func = ((int)addr == -1) ? -1 : addr & 0x0000FFFF;
- slot = ((int)addr == -1) ? -1 : (addr >> 16) & 0x0000FFFF;
- if ((int)ui_slot != slot) {
- g_debug("mismatch with reported slot values: %d vs %d",
- (int)ui_slot, slot);
- }
- pci->domain = 0;
- pci->slot = (int)ui_slot;
- pci->function = func;
- pci->bus = (int)bus;
- break;
- }
- }
break;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 4/6] gqa-win: get_pci_info: Split logic to separate functions
2021-12-14 12:41 ` [PATCH v2 4/6] gqa-win: get_pci_info: Split logic to separate functions Kostiantyn Kostiuk
@ 2021-12-14 13:27 ` Marc-André Lureau
0 siblings, 0 replies; 13+ messages in thread
From: Marc-André Lureau @ 2021-12-14 13:27 UTC (permalink / raw)
To: Kostiantyn Kostiuk; +Cc: Michael Roth, Philippe Mathieu-Daudé, qemu-devel
On Tue, Dec 14, 2021 at 4:41 PM Kostiantyn Kostiuk
<konstantin@daynix.com> wrote:
>
> Signed-off-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
> Signed-off-by: Kostiantyn Kostiuk <konstantin@daynix.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
thanks!
> ---
> qga/commands-win32.c | 161 +++++++++++++++++++++++--------------------
> 1 file changed, 87 insertions(+), 74 deletions(-)
>
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index f6de9e2676..8588fa8633 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -512,6 +512,92 @@ DEFINE_GUID(GUID_DEVINTERFACE_STORAGEPORT,
> 0x2accfe60L, 0xc130, 0x11d2, 0xb0, 0x82,
> 0x00, 0xa0, 0xc9, 0x1e, 0xfb, 0x8b);
>
> +static void get_pci_address_for_device(GuestPCIAddress *pci,
> + HDEVINFO dev_info)
> +{
> + SP_DEVINFO_DATA dev_info_data;
> + DWORD j;
> + DWORD size;
> + bool partial_pci = false;
> +
> + dev_info_data.cbSize = sizeof(SP_DEVINFO_DATA);
> +
> + for (j = 0;
> + SetupDiEnumDeviceInfo(dev_info, j, &dev_info_data);
> + j++) {
> + DWORD addr, bus, ui_slot, type;
> + int func, slot;
> + size = sizeof(DWORD);
> +
> + /*
> + * There is no need to allocate buffer in the next functions. The
> + * size is known and ULONG according to
> + * https://msdn.microsoft.com/en-us/library/windows/hardware/ff543095(v=vs.85).aspx
> + */
> + if (!SetupDiGetDeviceRegistryProperty(
> + dev_info, &dev_info_data, SPDRP_BUSNUMBER,
> + &type, (PBYTE)&bus, size, NULL)) {
> + debug_error("failed to get PCI bus");
> + bus = -1;
> + partial_pci = true;
> + }
> +
> + /*
> + * The function retrieves the device's address. This value will be
> + * transformed into device function and number
> + */
> + if (!SetupDiGetDeviceRegistryProperty(
> + dev_info, &dev_info_data, SPDRP_ADDRESS,
> + &type, (PBYTE)&addr, size, NULL)) {
> + debug_error("failed to get PCI address");
> + addr = -1;
> + partial_pci = true;
> + }
> +
> + /*
> + * This call returns UINumber of DEVICE_CAPABILITIES structure.
> + * This number is typically a user-perceived slot number.
> + */
> + if (!SetupDiGetDeviceRegistryProperty(
> + dev_info, &dev_info_data, SPDRP_UI_NUMBER,
> + &type, (PBYTE)&ui_slot, size, NULL)) {
> + debug_error("failed to get PCI slot");
> + ui_slot = -1;
> + partial_pci = true;
> + }
> +
> + /*
> + * SetupApi gives us the same information as driver with
> + * IoGetDeviceProperty. According to Microsoft:
> + *
> + * FunctionNumber = (USHORT)((propertyAddress) & 0x0000FFFF)
> + * DeviceNumber = (USHORT)(((propertyAddress) >> 16) & 0x0000FFFF)
> + * SPDRP_ADDRESS is propertyAddress, so we do the same.
> + *
> + * https://docs.microsoft.com/en-us/windows/desktop/api/setupapi/nf-setupapi-setupdigetdeviceregistrypropertya
> + */
> + if (partial_pci) {
> + pci->domain = -1;
> + pci->slot = -1;
> + pci->function = -1;
> + pci->bus = -1;
> + continue;
> + } else {
> + func = ((int)addr == -1) ? -1 : addr & 0x0000FFFF;
> + slot = ((int)addr == -1) ? -1 : (addr >> 16) & 0x0000FFFF;
> + if ((int)ui_slot != slot) {
> + g_debug("mismatch with reported slot values: %d vs %d",
> + (int)ui_slot, slot);
> + }
> + pci->domain = 0;
> + pci->slot = (int)ui_slot;
> + pci->function = func;
> + pci->bus = (int)bus;
> + return;
> + }
> + }
> +}
> +
> static GuestPCIAddress *get_pci_info(int number, Error **errp)
> {
> HDEVINFO dev_info = INVALID_HANDLE_VALUE;
> @@ -522,7 +608,6 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
> HANDLE dev_file;
> int i;
> GuestPCIAddress *pci = NULL;
> - bool partial_pci = false;
>
> pci = g_malloc0(sizeof(*pci));
> pci->domain = -1;
> @@ -545,7 +630,6 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
> STORAGE_DEVICE_NUMBER sdn;
> char *parent_dev_id = NULL;
> SP_DEVINFO_DATA parent_dev_info_data;
> - DWORD j;
> DWORD size = 0;
>
> g_debug("getting device path");
> @@ -672,79 +756,8 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
> goto end;
> }
>
> - for (j = 0;
> - SetupDiEnumDeviceInfo(parent_dev_info, j, &parent_dev_info_data);
> - j++) {
> - DWORD addr, bus, ui_slot, type;
> - int func, slot;
> + get_pci_address_for_device(pci, parent_dev_info);
>
> - /*
> - * There is no need to allocate buffer in the next functions. The
> - * size is known and ULONG according to
> - * https://msdn.microsoft.com/en-us/library/windows/hardware/ff543095(v=vs.85).aspx
> - */
> - if (!SetupDiGetDeviceRegistryProperty(
> - parent_dev_info, &parent_dev_info_data, SPDRP_BUSNUMBER,
> - &type, (PBYTE)&bus, size, NULL)) {
> - debug_error("failed to get PCI bus");
> - bus = -1;
> - partial_pci = true;
> - }
> -
> - /*
> - * The function retrieves the device's address. This value will be
> - * transformed into device function and number
> - */
> - if (!SetupDiGetDeviceRegistryProperty(
> - parent_dev_info, &parent_dev_info_data, SPDRP_ADDRESS,
> - &type, (PBYTE)&addr, size, NULL)) {
> - debug_error("failed to get PCI address");
> - addr = -1;
> - partial_pci = true;
> - }
> -
> - /*
> - * This call returns UINumber of DEVICE_CAPABILITIES structure.
> - * This number is typically a user-perceived slot number.
> - */
> - if (!SetupDiGetDeviceRegistryProperty(
> - parent_dev_info, &parent_dev_info_data, SPDRP_UI_NUMBER,
> - &type, (PBYTE)&ui_slot, size, NULL)) {
> - debug_error("failed to get PCI slot");
> - ui_slot = -1;
> - partial_pci = true;
> - }
> -
> - /*
> - * SetupApi gives us the same information as driver with
> - * IoGetDeviceProperty. According to Microsoft:
> - *
> - * FunctionNumber = (USHORT)((propertyAddress) & 0x0000FFFF)
> - * DeviceNumber = (USHORT)(((propertyAddress) >> 16) & 0x0000FFFF)
> - * SPDRP_ADDRESS is propertyAddress, so we do the same.
> - *
> - * https://docs.microsoft.com/en-us/windows/desktop/api/setupapi/nf-setupapi-setupdigetdeviceregistrypropertya
> - */
> - if (partial_pci) {
> - pci->domain = -1;
> - pci->slot = -1;
> - pci->function = -1;
> - pci->bus = -1;
> - continue;
> - } else {
> - func = ((int)addr == -1) ? -1 : addr & 0x0000FFFF;
> - slot = ((int)addr == -1) ? -1 : (addr >> 16) & 0x0000FFFF;
> - if ((int)ui_slot != slot) {
> - g_debug("mismatch with reported slot values: %d vs %d",
> - (int)ui_slot, slot);
> - }
> - pci->domain = 0;
> - pci->slot = (int)ui_slot;
> - pci->function = func;
> - pci->bus = (int)bus;
> - break;
> - }
> - }
> break;
> }
>
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 5/6] gqa-win: get_pci_info: Add g_autofree for few variables
2021-12-14 12:41 [PATCH v2 0/6] gqa-win: get_pci_info: Fix memory leak Kostiantyn Kostiuk
` (3 preceding siblings ...)
2021-12-14 12:41 ` [PATCH v2 4/6] gqa-win: get_pci_info: Split logic to separate functions Kostiantyn Kostiuk
@ 2021-12-14 12:41 ` Kostiantyn Kostiuk
2021-12-14 13:28 ` Marc-André Lureau
2021-12-14 12:41 ` [PATCH v2 6/6] gqa-win: get_pci_info: Replace 'while' with 2 calls of the function Kostiantyn Kostiuk
5 siblings, 1 reply; 13+ messages in thread
From: Kostiantyn Kostiuk @ 2021-12-14 12:41 UTC (permalink / raw)
To: qemu-devel, Michael Roth, Marc-André Lureau,
Philippe Mathieu-Daudé
Signed-off-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
Signed-off-by: Kostiantyn Kostiuk <konstantin@daynix.com>
---
qga/commands-win32.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 8588fa8633..3092566313 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -626,9 +626,9 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
dev_info_data.cbSize = sizeof(SP_DEVINFO_DATA);
dev_iface_data.cbSize = sizeof(SP_DEVICE_INTERFACE_DATA);
for (i = 0; SetupDiEnumDeviceInfo(dev_info, i, &dev_info_data); i++) {
- PSP_DEVICE_INTERFACE_DETAIL_DATA pdev_iface_detail_data = NULL;
+ g_autofree PSP_DEVICE_INTERFACE_DETAIL_DATA pdev_iface_detail_data = NULL;
STORAGE_DEVICE_NUMBER sdn;
- char *parent_dev_id = NULL;
+ g_autofree char *parent_dev_id = NULL;
SP_DEVINFO_DATA parent_dev_info_data;
DWORD size = 0;
@@ -654,7 +654,6 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
dev_file = CreateFile(pdev_iface_detail_data->DevicePath, 0,
FILE_SHARE_READ, NULL, OPEN_EXISTING, 0,
NULL);
- g_free(pdev_iface_detail_data);
if (!DeviceIoControl(dev_file, IOCTL_STORAGE_GET_DEVICE_NUMBER,
NULL, 0, &sdn, sizeof(sdn), &size, NULL)) {
@@ -741,7 +740,6 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
parent_dev_info =
SetupDiGetClassDevs(&GUID_DEVINTERFACE_STORAGEPORT, parent_dev_id,
NULL, DIGCF_PRESENT | DIGCF_DEVICEINTERFACE);
- g_free(parent_dev_id);
if (parent_dev_info == INVALID_HANDLE_VALUE) {
error_setg_win32(errp, GetLastError(),
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 5/6] gqa-win: get_pci_info: Add g_autofree for few variables
2021-12-14 12:41 ` [PATCH v2 5/6] gqa-win: get_pci_info: Add g_autofree for few variables Kostiantyn Kostiuk
@ 2021-12-14 13:28 ` Marc-André Lureau
0 siblings, 0 replies; 13+ messages in thread
From: Marc-André Lureau @ 2021-12-14 13:28 UTC (permalink / raw)
To: Kostiantyn Kostiuk; +Cc: Michael Roth, Philippe Mathieu-Daudé, qemu-devel
On Tue, Dec 14, 2021 at 4:41 PM Kostiantyn Kostiuk
<konstantin@daynix.com> wrote:
>
> Signed-off-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
> Signed-off-by: Kostiantyn Kostiuk <konstantin@daynix.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> qga/commands-win32.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index 8588fa8633..3092566313 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -626,9 +626,9 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
> dev_info_data.cbSize = sizeof(SP_DEVINFO_DATA);
> dev_iface_data.cbSize = sizeof(SP_DEVICE_INTERFACE_DATA);
> for (i = 0; SetupDiEnumDeviceInfo(dev_info, i, &dev_info_data); i++) {
> - PSP_DEVICE_INTERFACE_DETAIL_DATA pdev_iface_detail_data = NULL;
> + g_autofree PSP_DEVICE_INTERFACE_DETAIL_DATA pdev_iface_detail_data = NULL;
> STORAGE_DEVICE_NUMBER sdn;
> - char *parent_dev_id = NULL;
> + g_autofree char *parent_dev_id = NULL;
> SP_DEVINFO_DATA parent_dev_info_data;
> DWORD size = 0;
>
> @@ -654,7 +654,6 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
> dev_file = CreateFile(pdev_iface_detail_data->DevicePath, 0,
> FILE_SHARE_READ, NULL, OPEN_EXISTING, 0,
> NULL);
> - g_free(pdev_iface_detail_data);
>
> if (!DeviceIoControl(dev_file, IOCTL_STORAGE_GET_DEVICE_NUMBER,
> NULL, 0, &sdn, sizeof(sdn), &size, NULL)) {
> @@ -741,7 +740,6 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
> parent_dev_info =
> SetupDiGetClassDevs(&GUID_DEVINTERFACE_STORAGEPORT, parent_dev_id,
> NULL, DIGCF_PRESENT | DIGCF_DEVICEINTERFACE);
> - g_free(parent_dev_id);
>
> if (parent_dev_info == INVALID_HANDLE_VALUE) {
> error_setg_win32(errp, GetLastError(),
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 6/6] gqa-win: get_pci_info: Replace 'while' with 2 calls of the function
2021-12-14 12:41 [PATCH v2 0/6] gqa-win: get_pci_info: Fix memory leak Kostiantyn Kostiuk
` (4 preceding siblings ...)
2021-12-14 12:41 ` [PATCH v2 5/6] gqa-win: get_pci_info: Add g_autofree for few variables Kostiantyn Kostiuk
@ 2021-12-14 12:41 ` Kostiantyn Kostiuk
2021-12-14 13:28 ` Marc-André Lureau
5 siblings, 1 reply; 13+ messages in thread
From: Kostiantyn Kostiuk @ 2021-12-14 12:41 UTC (permalink / raw)
To: qemu-devel, Michael Roth, Marc-André Lureau,
Philippe Mathieu-Daudé
Microsoft suggests this solution in the documentation:
https://docs.microsoft.com/en-us/windows/win32/api/setupapi/nf-setupapi-setupdigetdeviceinterfacedetaila
Signed-off-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
Signed-off-by: Kostiantyn Kostiuk <konstantin@daynix.com>
---
qga/commands-win32.c | 30 ++++++++++++++++++++++++------
1 file changed, 24 insertions(+), 6 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 3092566313..892082504f 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -636,10 +636,10 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
if (SetupDiEnumDeviceInterfaces(dev_info, &dev_info_data,
&GUID_DEVINTERFACE_DISK, 0,
&dev_iface_data)) {
- while (!SetupDiGetDeviceInterfaceDetail(dev_info, &dev_iface_data,
- pdev_iface_detail_data,
- size, &size,
- &dev_info_data)) {
+ if (!SetupDiGetDeviceInterfaceDetail(dev_info, &dev_iface_data,
+ pdev_iface_detail_data,
+ size, &size,
+ &dev_info_data)) {
if (GetLastError() == ERROR_INSUFFICIENT_BUFFER) {
pdev_iface_detail_data = g_malloc(size);
pdev_iface_detail_data->cbSize =
@@ -651,6 +651,16 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
}
}
+ if (!SetupDiGetDeviceInterfaceDetail(dev_info, &dev_iface_data,
+ pdev_iface_detail_data,
+ size, &size,
+ &dev_info_data)) {
+ // pdev_iface_detail_data already is allocated
+ error_setg_win32(errp, GetLastError(),
+ "failed to get device interfaces");
+ goto end;
+ }
+
dev_file = CreateFile(pdev_iface_detail_data->DevicePath, 0,
FILE_SHARE_READ, NULL, OPEN_EXISTING, 0,
NULL);
@@ -680,8 +690,8 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
ULONG dev_id_size = 0;
size = 0;
- while (!SetupDiGetDeviceInstanceId(dev_info, &dev_info_data,
- parent_dev_id, size, &size)) {
+ if (!SetupDiGetDeviceInstanceId(dev_info, &dev_info_data,
+ parent_dev_id, size, &size)) {
if (GetLastError() == ERROR_INSUFFICIENT_BUFFER) {
parent_dev_id = g_malloc(size);
} else {
@@ -691,6 +701,14 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
}
}
+ if (!SetupDiGetDeviceInstanceId(dev_info, &dev_info_data,
+ parent_dev_id, size, &size)) {
+ // parent_dev_id already is allocated
+ error_setg_win32(errp, GetLastError(),
+ "failed to get device instance ID");
+ goto end;
+ }
+
/*
* CM API used here as opposed to
* SetupDiGetDeviceProperty(..., DEVPKEY_Device_Parent, ...)
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 6/6] gqa-win: get_pci_info: Replace 'while' with 2 calls of the function
2021-12-14 12:41 ` [PATCH v2 6/6] gqa-win: get_pci_info: Replace 'while' with 2 calls of the function Kostiantyn Kostiuk
@ 2021-12-14 13:28 ` Marc-André Lureau
0 siblings, 0 replies; 13+ messages in thread
From: Marc-André Lureau @ 2021-12-14 13:28 UTC (permalink / raw)
To: Kostiantyn Kostiuk; +Cc: Michael Roth, Philippe Mathieu-Daudé, qemu-devel
On Tue, Dec 14, 2021 at 4:41 PM Kostiantyn Kostiuk
<konstantin@daynix.com> wrote:
>
> Microsoft suggests this solution in the documentation:
> https://docs.microsoft.com/en-us/windows/win32/api/setupapi/nf-setupapi-setupdigetdeviceinterfacedetaila
>
> Signed-off-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
> Signed-off-by: Kostiantyn Kostiuk <konstantin@daynix.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> qga/commands-win32.c | 30 ++++++++++++++++++++++++------
> 1 file changed, 24 insertions(+), 6 deletions(-)
>
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index 3092566313..892082504f 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -636,10 +636,10 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
> if (SetupDiEnumDeviceInterfaces(dev_info, &dev_info_data,
> &GUID_DEVINTERFACE_DISK, 0,
> &dev_iface_data)) {
> - while (!SetupDiGetDeviceInterfaceDetail(dev_info, &dev_iface_data,
> - pdev_iface_detail_data,
> - size, &size,
> - &dev_info_data)) {
> + if (!SetupDiGetDeviceInterfaceDetail(dev_info, &dev_iface_data,
> + pdev_iface_detail_data,
> + size, &size,
> + &dev_info_data)) {
> if (GetLastError() == ERROR_INSUFFICIENT_BUFFER) {
> pdev_iface_detail_data = g_malloc(size);
> pdev_iface_detail_data->cbSize =
> @@ -651,6 +651,16 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
> }
> }
>
> + if (!SetupDiGetDeviceInterfaceDetail(dev_info, &dev_iface_data,
> + pdev_iface_detail_data,
> + size, &size,
> + &dev_info_data)) {
> + // pdev_iface_detail_data already is allocated
> + error_setg_win32(errp, GetLastError(),
> + "failed to get device interfaces");
> + goto end;
> + }
> +
> dev_file = CreateFile(pdev_iface_detail_data->DevicePath, 0,
> FILE_SHARE_READ, NULL, OPEN_EXISTING, 0,
> NULL);
> @@ -680,8 +690,8 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
> ULONG dev_id_size = 0;
>
> size = 0;
> - while (!SetupDiGetDeviceInstanceId(dev_info, &dev_info_data,
> - parent_dev_id, size, &size)) {
> + if (!SetupDiGetDeviceInstanceId(dev_info, &dev_info_data,
> + parent_dev_id, size, &size)) {
> if (GetLastError() == ERROR_INSUFFICIENT_BUFFER) {
> parent_dev_id = g_malloc(size);
> } else {
> @@ -691,6 +701,14 @@ static GuestPCIAddress *get_pci_info(int number, Error **errp)
> }
> }
>
> + if (!SetupDiGetDeviceInstanceId(dev_info, &dev_info_data,
> + parent_dev_id, size, &size)) {
> + // parent_dev_id already is allocated
> + error_setg_win32(errp, GetLastError(),
> + "failed to get device instance ID");
> + goto end;
> + }
> +
> /*
> * CM API used here as opposed to
> * SetupDiGetDeviceProperty(..., DEVPKEY_Device_Parent, ...)
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 13+ messages in thread