Dont ignore failures reported in OnEnd when doing bulk property queries

kovidgoyal · Jan 25, 2025 · 66cf52e · 66cf52e
1 parent f1d2910
commit 66cf52e
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 9 deletions.
diff --git a/src/calibre/devices/mtp/windows/content_enumeration.cpp b/src/calibre/devices/mtp/windows/content_enumeration.cpp
@@ -157,6 +157,7 @@ class GetBulkPropertiesCallback : public IPortableDevicePropertiesBulkCallback {
     HANDLE complete;
     ULONG self_ref;
     PyObject *callback;
+    HRESULT end_status;
 
 	void do_one_object(CComPtr<IPortableDeviceValues> &properties) {
 		com_wchar_raii property;
@@ -200,17 +201,26 @@ class GetBulkPropertiesCallback : public IPortableDevicePropertiesBulkCallback {
 		if (complete == NULL || complete == INVALID_HANDLE_VALUE) return false;
 
 		this->items = items; this->subfolders = subfolders; this->level = level; this->callback = callback;
+        this->end_status = S_OK;
 		self_ref = 0;
 		return true;
 	}
-	void end_processing() {
+	HRESULT end_processing() {
 		if (complete != INVALID_HANDLE_VALUE) CloseHandle(complete);
 		items = NULL; subfolders = NULL; level = 0; complete = INVALID_HANDLE_VALUE; callback = NULL;
+        return this->end_status;
 	}
+
 	bool handle_is_valid() const { return complete != INVALID_HANDLE_VALUE; }
 
     HRESULT __stdcall OnStart(REFGUID Context) { return S_OK; }
-    HRESULT __stdcall OnEnd(REFGUID Context, HRESULT hrStatus) { if (complete != INVALID_HANDLE_VALUE) SetEvent(complete); return S_OK; }
+
+    HRESULT __stdcall OnEnd(REFGUID Context, HRESULT hrStatus) {
+        if (complete != INVALID_HANDLE_VALUE) SetEvent(complete);
+        this->end_status = hrStatus;
+        return S_OK;
+    }
+
     ULONG __stdcall AddRef() { InterlockedIncrement((long*) &self_ref); return self_ref; }
     ULONG __stdcall Release() {
         ULONG refcnt = self_ref - 1;
@@ -304,13 +314,18 @@ bulk_get_filesystem(
             PyErr_SetExcFromWindowsErrWithFilename(WPDError, 0, buf);
         }
     }
-    bulk_properties_callback->end_processing();
+    hr = bulk_properties_callback->end_processing();
     if (PyErr_Occurred()) {
         bulk_properties->Cancel(guid_context);
         pump_waiting_messages();
     }
 	bulk_properties_callback->Release();
-    return PyErr_Occurred() ? false : true;
+    if (PyErr_Occurred()) return false;
+    if (FAILED(hr)) {
+        hresult_set_exc("Bulk get properties failed in OnEnd", hr);
+        return false;
+    }
+    return true;
 }
 
 // }}}

diff --git a/src/calibre/devices/mtp/windows/device.cpp b/src/calibre/devices/mtp/windows/device.cpp
@@ -36,7 +36,7 @@ init(Device *self, PyObject *args, PyObject *kwds)
     if (client_information) {
         self->device = open_device(self->pnp_id.ptr(), client_information);
         if (self->device) {
-            self->device_information.attach(get_device_information(self->device, self->bulk_properties));
+            self->device_information.attach(get_device_information(self->pnp_id.ptr(), self->device, self->bulk_properties));
             if (self->device_information) ret = 0;
         }
     }
@@ -50,7 +50,7 @@ static PyObject*
 update_data(Device *self, PyObject *args) {
     PyObject *di = NULL;
     CComPtr<IPortableDevicePropertiesBulk> bulk_properties;
-    di = get_device_information(self->device, bulk_properties);
+    di = get_device_information(self->pnp_id.ptr(), self->device, bulk_properties);
     if (di == NULL) return NULL;
     self->device_information.attach(di);
     Py_RETURN_NONE;

diff --git a/src/calibre/devices/mtp/windows/device_enumeration.cpp b/src/calibre/devices/mtp/windows/device_enumeration.cpp
@@ -151,7 +151,7 @@ get_storage_info(IPortableDevice *device) { // {{{
 } // }}}
 
 PyObject*
-get_device_information(CComPtr<IPortableDevice> &device, CComPtr<IPortableDevicePropertiesBulk> &pb) { // {{{
+get_device_information(const wchar_t *pnp_id, CComPtr<IPortableDevice> &device, CComPtr<IPortableDevicePropertiesBulk> &pb) { // {{{
     CComPtr<IPortableDeviceContent> content = NULL;
     CComPtr<IPortableDeviceProperties> properties = NULL;
     CComPtr<IPortableDeviceKeyCollection> keys = NULL;
@@ -260,6 +260,8 @@ get_device_information(CComPtr<IPortableDevice> &device, CComPtr<IPortableDevice
         }
     }
     PyDict_SetItemString(ans, "has_storage", has_storage ? Py_True : Py_False);
+    pyobject_raii pid(PyUnicode_FromWideChar(pnp_id, -1));
+    if (pid) PyDict_SetItemString(ans, "pnp_id", pid.ptr());
 
     if (has_storage) {
         pyobject_raii storage(get_storage_info(device));

diff --git a/src/calibre/devices/mtp/windows/global.h b/src/calibre/devices/mtp/windows/global.h
@@ -43,7 +43,7 @@ extern PyTypeObject DeviceType;
 
 extern IPortableDeviceValues* get_client_information();
 extern IPortableDevice* open_device(const wchar_t *pnp_id, CComPtr<IPortableDeviceValues> &client_information);
-extern PyObject* get_device_information(CComPtr<IPortableDevice> &device, CComPtr<IPortableDevicePropertiesBulk> &bulk_properties);
+extern PyObject* get_device_information(const wchar_t *pnp_id, CComPtr<IPortableDevice> &device, CComPtr<IPortableDevicePropertiesBulk> &bulk_properties);
 extern PyObject* get_filesystem(IPortableDevice *device, const wchar_t *storage_id, IPortableDevicePropertiesBulk *bulk_properties, PyObject *callback);
 extern PyObject* get_file(IPortableDevice *device, const wchar_t *object_id, PyObject *dest, PyObject *callback);
 extern PyObject* create_folder(IPortableDevice *device, const wchar_t *parent_id, const wchar_t *name);

diff --git a/src/calibre/devices/mtp/windows/wpd.cpp b/src/calibre/devices/mtp/windows/wpd.cpp
@@ -185,7 +185,7 @@ wpd_device_info(PyObject *self, PyObject *args) {
     if (client_information) {
         device = open_device(pnp_id.ptr(), client_information);
         CComPtr<IPortableDevicePropertiesBulk> properties_bulk;
-        if (device) ans = get_device_information(device, properties_bulk);
+        if (device) ans = get_device_information(pnp_id.ptr(), device, properties_bulk);
     }
 
     if (device) device->Close();