Change comments to not use American expressions. Got to matching base…

…d on the PDO name. Tie PnP callback removal to the lifetime of the device object.
forderud · Mar 14, 2024 · d9f4fed · d9f4fed
1 parent a002980
commit d9f4fed
Show file tree

Hide file tree

Showing 5 changed files with 53 additions and 67 deletions.
diff --git a/TailLight/SetTaillightBlack.cpp b/TailLight/SetTaillightBlack.cpp
@@ -36,7 +36,7 @@ NTSTATUS CreateWorkItemForIoTargetOpenDevice(WDFDEVICE device,
 
         DEVICE_CONTEXT* pDeviceContext = WdfObjectGet_DEVICE_CONTEXT(device);
 
-        // It's possible to get called twice. Been there, done that?
+        // It's possible to get called twice.
         if ((!pDeviceContext) || pDeviceContext->fSetBlackSuccess) {
             return STATUS_SUCCESS;
         }
@@ -105,7 +105,7 @@ void SetBlackCompletionRoutine(
     status = WdfRequestGetStatus(Request);
     KdPrint(("TailLight: %s WdfRequestSend status: 0x%x\n", __func__, status));
 
-    // One-shot and top of stack, so delete and pray.
+    // One-shot and top of stack, so delete.
     WdfObjectDelete(Request);
 }
 
@@ -134,7 +134,7 @@ VOID SetBlackWorkItem(
         InterlockedIncrement((PLONG)(&pDeviceContext->fSetBlackSuccess));
     }
 
-    NukeWdfHandle<WDFWORKITEM>(workItem);
+    WdfObjectDelete(workItem);
 }
 
 NTSTATUS SetBlackAsync(WDFDEVICE device, 
@@ -171,20 +171,25 @@ NTSTATUS SetBlackAsync(WDFDEVICE device,
             return status;
         }
 
+        // We're warned not to process a symlink path name returned from 
+        // IoRegisterDeviceInterface so we need to use a unique device property
+        //  - the device object name - to see if we can send the IRP down with
+        //  minimal bus traffic.
         status = TryToOpenIoTarget(hidTarget, symLink);
     }
 
     if (NT_SUCCESS(status)) {
 
-        UNICODE_STRING theirHwId = {};
+        UNICODE_STRING theirPDOName = {};
 
-        // The hardware property ID will be attached to the IO target
+        // The PDO name will be attached to the IO target
         // and thus deleted at the end of the function.
-        theirHwId = GetTargetPropertyString(hidTarget,
-            DevicePropertyHardwareID);
-        if (NT_SUCCESS(status)) {
-            if (!RtlEqualUnicodeString(&pDeviceContext->HardwareId,
-                &theirHwId,
+        theirPDOName = GetTargetPropertyString(hidTarget,
+            DevicePropertyPhysicalDeviceObjectName);
+
+        if (theirPDOName.MaximumLength > 0) {
+            if (!RtlEqualUnicodeString(&pDeviceContext->PdoName,
+                &theirPDOName,
                 TRUE)) {
                 status = STATUS_NOT_FOUND;
                 goto ExitAndFree;
@@ -257,7 +262,7 @@ NTSTATUS SetBlackAsync(WDFDEVICE device,
     }
 
 ExitAndFree:
-    NukeWdfHandle<WDFIOTARGET>(hidTarget);
+    WdfObjectDelete(hidTarget);
 
     return status;
 }
diff --git a/TailLight/device.cpp b/TailLight/device.cpp
@@ -29,7 +29,8 @@ NTSTATUS PnpNotifyDeviceInterfaceChange(
 
             // Opening a device may trigger PnP operations. Ensure that either a
             // timer or a work item is used when opening up a device.
-            // Refer to p356 of Oney and IoGetDeviceObjectPointer.
+            // Refer to page 356 of "Programming The Microsoft Windows Driver
+            // Model", 2nd edition by Walter Oney and IoGetDeviceObjectPointer.
             //
             // NOTE: It is possible for us to get blocked waiting for a system
             // thread. One solution would be to use a timer that spawns a
@@ -49,9 +50,7 @@ NTSTATUS PnpNotifyDeviceInterfaceChange(
 NTSTATUS EvtSelfManagedIoInit(WDFDEVICE device) {
 
     WDFDRIVER driver = WdfDeviceGetDriver(device);
-    DRIVER_CONTEXT* driverContext = WdfObjectGet_DRIVER_CONTEXT(driver);
-
-    //TRACE_FN_ENTRY;
+    DEVICE_CONTEXT* pDeviceContext = WdfObjectGet_DEVICE_CONTEXT(device);
 
     NTSTATUS status = STATUS_SUCCESS;
 
@@ -62,7 +61,7 @@ NTSTATUS EvtSelfManagedIoInit(WDFDEVICE device) {
         WdfDriverWdmGetDriverObject(driver),
         PnpNotifyDeviceInterfaceChange,
         (PVOID)device,
-        &driverContext->pnpDevInterfaceChangedHandle
+        &pDeviceContext->pnpDevInterfaceChangedHandle
     );
 
     //TRACE_FN_EXIT;
@@ -96,6 +95,35 @@ UNICODE_STRING GetTargetPropertyString(WDFIOTARGET target, DEVICE_REGISTRY_PROPE
 }
 
 
+VOID EvtCleanupCallback(
+    _In_ WDFOBJECT object
+)
+/*++
+Routine Description:
+    Removes the plug and play notification callback if registered.
+    Called near the end of processing an IRP_MN_REMOVE_DEVICE.
+    This work could also be done at EvtDeviceSelfManagedIoCleanup, which
+    comes before this callback. However using the lifetime of the device
+    context means there is little chance of the device context disappearing
+    from underneath along with the pnp notification handle.
+
+Arguments:
+    WDFOBJECT - Handle to a framework device object from AddDevice
+--*/
+{
+    DEVICE_CONTEXT* pDeviceContext = WdfObjectGet_DEVICE_CONTEXT(object);
+
+    if (pDeviceContext->pnpDevInterfaceChangedHandle) {
+        NTSTATUS status = STATUS_SUCCESS;
+        status = IoUnregisterPlugPlayNotificationEx(pDeviceContext->pnpDevInterfaceChangedHandle);
+        if (!NT_SUCCESS(status)) {
+            KdPrint(("IoUnregisterPlugPlayNotification failed with 0x%x\n", status));
+        }
+    }
+    pDeviceContext->pnpDevInterfaceChangedHandle = NULL;
+}
+
+
 NTSTATUS EvtDriverDeviceAdd(_In_ WDFDRIVER Driver, _Inout_ PWDFDEVICE_INIT DeviceInit)
 /*++
 Routine Description:
@@ -127,6 +155,7 @@ Routine Description:
         // create device
         WDF_OBJECT_ATTRIBUTES attributes = {};
         WDF_OBJECT_ATTRIBUTES_INIT_CONTEXT_TYPE(&attributes, DEVICE_CONTEXT);
+        attributes.EvtCleanupCallback = EvtCleanupCallback;
 
         NTSTATUS status = WdfDeviceCreate(&DeviceInit, &attributes, &device);
         if (!NT_SUCCESS(status)) {
@@ -148,17 +177,6 @@ Routine Description:
 
         KdPrint(("TailLight: PdoName: %wZ\n", deviceContext->PdoName)); // outputs "\Device\00000083"
     }
-    {
-        // initialize DEVICE_CONTEXT struct with HardwareId
-        deviceContext->HardwareId = GetTargetPropertyString(WdfDeviceGetIoTarget(device), DevicePropertyHardwareID);
-        if (!deviceContext->HardwareId.Buffer) {
-            KdPrint(("TailLight: HardwareId query failed\n"));
-            return STATUS_UNSUCCESSFUL;
-        }
-
-        KdPrint(("TailLight: HardwareId: %wZ\n", deviceContext->HardwareId));
-    }
-
     {
         // create queue for filtering
         WDF_IO_QUEUE_CONFIG queueConfig = {};
@@ -239,4 +257,4 @@ Routine Description:
         KdPrint(("TailLight: WdfRequestSend failed with status: 0x%x\n", status));
         WdfRequestComplete(Request, status);
     }
-}
+}
diff --git a/TailLight/device.h b/TailLight/device.h
@@ -5,17 +5,10 @@ struct DEVICE_CONTEXT {
     UNICODE_STRING PdoName;
     BOOLEAN        fSetBlackSuccess;
     WDFWMIINSTANCE WmiInstance;
-    UNICODE_STRING HardwareId;
+    PVOID          pnpDevInterfaceChangedHandle;
 };
 
 
-template<typename T> inline void NukeWdfHandle(T& handle) {
-    if (handle) {
-        WdfObjectDelete(handle);
-        handle = 0;
-    }
-}
-
 WDF_DECLARE_CONTEXT_TYPE(DEVICE_CONTEXT)
 
 WDF_DECLARE_CONTEXT_TYPE(TailLightDeviceInformation)

diff --git a/TailLight/driver.cpp b/TailLight/driver.cpp
@@ -12,15 +12,11 @@ NTSTATUS DriverEntry(
     WDF_DRIVER_CONFIG params = {};
     WDF_DRIVER_CONFIG_INIT(/*out*/&params, EvtDriverDeviceAdd);
     params.DriverPoolTag = POOL_TAG;
-    params.EvtDriverUnload = EvtDriverUnload;
-
-    WDF_OBJECT_ATTRIBUTES attributes = {};
-    WDF_OBJECT_ATTRIBUTES_INIT_CONTEXT_TYPE(&attributes, DRIVER_CONTEXT);
 
     // Create the framework WDFDRIVER object, with the handle to it returned in Driver.
     NTSTATUS status = WdfDriverCreate(DriverObject, 
                              RegistryPath, 
-                             &attributes, 
+                             WDF_NO_OBJECT_ATTRIBUTES, 
                              &params, 
                              WDF_NO_HANDLE); // [out]
     if (!NT_SUCCESS(status)) {
@@ -31,23 +27,3 @@ NTSTATUS DriverEntry(
     return status;
 }
 
-
-/** Driver unload callback.
-    Used to perform operations that must take place before the driver is unloaded.  */
-VOID EvtDriverUnload(
-    _In_ WDFDRIVER Driver
-    )
-{
-    KdPrint(("TailLight: DriverUnload.\n"));
-
-    DRIVER_CONTEXT* pDriverContext = WdfObjectGet_DRIVER_CONTEXT(Driver);
-
-    if (pDriverContext->pnpDevInterfaceChangedHandle) {
-        NTSTATUS status = STATUS_SUCCESS;
-        status = IoUnregisterPlugPlayNotificationEx(pDriverContext->pnpDevInterfaceChangedHandle);
-        if (!NT_SUCCESS(status)) {
-            KdPrint(("IoUnregisterPlugPlayNotification failed with 0x%x\n", status));
-        }
-    }
-    pDriverContext->pnpDevInterfaceChangedHandle = NULL;
-}
diff --git a/TailLight/driver.h b/TailLight/driver.h
@@ -16,12 +16,6 @@
 /** Memory allocation tag name (for debugging leaks). */
 static constexpr ULONG POOL_TAG = 'iLaT'; // displayed as "TaLi"
 
-struct DRIVER_CONTEXT {
-    PVOID pnpDevInterfaceChangedHandle;
-};
-
-WDF_DECLARE_CONTEXT_TYPE(DRIVER_CONTEXT);
-
 extern "C"
 DRIVER_INITIALIZE         DriverEntry;