[PATCH] winedevice: Remove static driver_obj variable

[Aric Stewart]

On 8/5/16 10:04 AM, Sebastian Lackner wrote:
> On 05.08.2016 14:26, Aric Stewart wrote:
>> Signed-off-by: Aric Stewart <aric at codeweavers.com>
>> ---
>>  programs/winedevice/device.c | 36 ++++++++++++++++++++++++++++++------
>>  1 file changed, 30 insertions(+), 6 deletions(-)
>>
>>
>>
>> 0001-winedevice-Remove-static-driver_obj-variable.txt
>>
>>
>> diff --git a/programs/winedevice/device.c b/programs/winedevice/device.c
>> index e31318c..2744b9d5 100644
>> --- a/programs/winedevice/device.c
>> +++ b/programs/winedevice/device.c
>> @@ -42,7 +42,16 @@ extern NTSTATUS CDECL wine_ntoskrnl_main_loop( HANDLE stop_event );
>>  
>>  static SERVICE_STATUS_HANDLE service_handle;
>>  static HANDLE stop_event;
>> -static DRIVER_OBJECT *driver_obj;
>> +
>> +static const WCHAR driverW[] = {'\\','D','r','i','v','e','r','\\',0};
>> +extern NTSTATUS WINAPI ObReferenceObjectByName( UNICODE_STRING *ObjectName,
>> +                                         ULONG Attributes,
>> +                                         ACCESS_STATE *AccessState,
>> +                                         ACCESS_MASK DesiredAccess,
>> +                                         POBJECT_TYPE ObjectType,
>> +                                         KPROCESSOR_MODE AccessMode,
>> +                                         void *ParseContext,
>> +                                         void **Object);
> 
> It might be better to add this to a public header file. On Windows its in a
> different header file, but Wine already has a couple of ObReference* functions
> in include/ddk/wdm.h. You could add it there, too.
> 
>>  
>>  /* find the LDR_MODULE corresponding to the driver module */
>>  static LDR_MODULE *find_ldr_module( HMODULE module )
>> @@ -225,7 +234,6 @@ static NTSTATUS WINAPI init_driver( DRIVER_OBJECT *driver_object, UNICODE_STRING
>>      if (!module)
>>          return STATUS_DLL_INIT_FAILED;
>>  
>> -    driver_obj = driver_object;
>>      driver_object->DriverSection = find_ldr_module( module );
>>  
>>      nt = RtlImageNtHeader( module );
>> @@ -253,9 +261,26 @@ static NTSTATUS WINAPI init_driver( DRIVER_OBJECT *driver_object, UNICODE_STRING
>>  }
>>  
>>  /* call the driver unload function */
>> -static void unload_driver( DRIVER_OBJECT *driver_obj )
>> +static void unload_driver( const WCHAR *driver_name )
>>  {
>> -    LDR_MODULE *ldr = driver_obj->DriverSection;
>> +    DRIVER_OBJECT *driver_obj;
>> +    LDR_MODULE *ldr;
>> +    UNICODE_STRING drv_name;
>> +    WCHAR *str;
>> +
>> +    str = HeapAlloc( GetProcessHeap(), 0, sizeof(driverW) + strlenW(driver_name)*sizeof(WCHAR) );
>> +    lstrcpyW( str, driverW);
>> +    lstrcatW( str, driver_name );
>> +    RtlInitUnicodeString( &drv_name, str );
>> +
>> +    if (ObReferenceObjectByName( &drv_name, OBJ_CASE_INSENSITIVE, NULL,
>> +        0, NULL, KernelMode, NULL, (void**)&driver_obj ) != STATUS_SUCCESS)
> 
> To avoid putting together the string twice, you could already call this function
> immediately after loading.
> 

I dont quite understand what you are meaning here...

-aric




> Also, in theory you would have to release the reference obtained by
> "ObReferenceObjectByName" using ObDereferenceObject. Since there is no refcounting
> yet its fine to skip it, but it might be important at some later point.
> 
>> +    {
>> +        ERR("Failed to locate loaded driver (%s)\n", wine_dbgstr_w(driver_name));
>> +        return;
> 
> You are leaking the drv_name variable here and at the end of the function.
> 
>> +    }
>> +
>> +    ldr = driver_obj->DriverSection;
>>  
>>      if (driver_obj->DriverUnload)
>>      {
>> @@ -276,7 +301,6 @@ static void unload_driver( DRIVER_OBJECT *driver_obj )
>>  
>>  static NTSTATUS create_driver(const WCHAR *driver_name)
>>  {
>> -    static const WCHAR driverW[] = {'\\','D','r','i','v','e','r','\\',0};
>>      UNICODE_STRING drv_name;
>>      NTSTATUS status;
>>      WCHAR *str;
>> @@ -351,7 +375,7 @@ static void WINAPI ServiceMain( DWORD argc, LPWSTR *argv )
>>          SetServiceStatus( service_handle, &status );
>>  
>>          wine_ntoskrnl_main_loop( stop_event );
>> -        unload_driver( driver_obj );
>> +        unload_driver( driver_name );
>>      }
>>      else WINE_ERR( "driver %s failed to load\n", wine_dbgstr_w(driver_name) );
>>  
>>
>>
>>
>