[prev in list] [next in list] [prev in thread] [next in thread] 

List:       wine-devel
Subject:    Re: [PATCH] ntoskrnl.exe: Correctly initialize two CRITICAL_SECTION_DEBUGs
From:       Michael Stefaniuc <mstefani () winehq ! org>
Date:       2019-04-30 19:58:15
Message-ID: c7f35e8e-7af2-9922-4bd7-e59b5478a526 () winehq ! org
[Download RAW message or body]

On 4/29/19 9:56 PM, Jacek Caban wrote:
> Jacek Caban <jacek@codeweavers.com>
> 
> On 4/29/19 9:13 PM, Michael Stefaniuc wrote:
>> Signed-off-by: Michael Stefaniuc <mstefani@winehq.org>
>> ---
>>   dlls/ntoskrnl.exe/ntoskrnl.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/dlls/ntoskrnl.exe/ntoskrnl.c b/dlls/ntoskrnl.exe/ntoskrnl.c
>> index b353839c0a..73fe054259 100644
>> --- a/dlls/ntoskrnl.exe/ntoskrnl.c
>> +++ b/dlls/ntoskrnl.exe/ntoskrnl.c
>> @@ -293,7 +293,7 @@ static CRITICAL_SECTION obref_cs;
>>   static CRITICAL_SECTION_DEBUG obref_critsect_debug =
>>   {
>>       0, 0, &obref_cs,
>> -    { &critsect_debug.ProcessLocksList,
>> &critsect_debug.ProcessLocksList },
>> +    { &obref_critsect_debug.ProcessLocksList,
>> &obref_critsect_debug.ProcessLocksList },
>>         0, 0, { (DWORD_PTR)(__FILE__ ": obref_cs") }
>>   };
>>   static CRITICAL_SECTION obref_cs = { &obref_critsect_debug, -1, 0,
>> 0, 0, 0 };
>> @@ -396,7 +396,7 @@ static CRITICAL_SECTION handle_map_cs;
>>   static CRITICAL_SECTION_DEBUG handle_map_critsect_debug =
>>   {
>>       0, 0, &handle_map_cs,
>> -    { &critsect_debug.ProcessLocksList,
>> &critsect_debug.ProcessLocksList },
>> +    { &handle_map_critsect_debug.ProcessLocksList,
>> &handle_map_critsect_debug.ProcessLocksList },
>>         0, 0, { (DWORD_PTR)(__FILE__ ": handle_map_cs") }
>>   };
>>   static CRITICAL_SECTION handle_map_cs = {
>> &handle_map_critsect_debug, -1, 0, 0, 0, 0 };
> 
> 
> FWIW, it would be nice if declaring a critical section would be easier
> and less error prone. Maybe WINE_STATIC_CRITICAL_SECTION() macro?
There's a reason I wrote the coccinelle script in the first place ;)

Anyway, I have attached an example patch. Once I have a working
coccinelle script to do the transformation I can send it in.

DECLARE_CRITICAL_SECTION() is IMHO a better name, can be prefixed with
WINE_ if so desired.

bye
	michael


["cs.diff" (text/x-patch)]

diff --git a/dlls/ntoskrnl.exe/ntoskrnl.c b/dlls/ntoskrnl.exe/ntoskrnl.c
index d8756a8685..fcade73c0a 100644
--- a/dlls/ntoskrnl.exe/ntoskrnl.c
+++ b/dlls/ntoskrnl.exe/ntoskrnl.c
@@ -392,14 +392,7 @@ static const POBJECT_TYPE *known_types[] =
     &SeTokenObjectType
 };
 
-static CRITICAL_SECTION handle_map_cs;
-static CRITICAL_SECTION_DEBUG handle_map_critsect_debug =
-{
-    0, 0, &handle_map_cs,
-    { &handle_map_critsect_debug.ProcessLocksList, &handle_map_critsect_debug.ProcessLocksList },
-      0, 0, { (DWORD_PTR)(__FILE__ ": handle_map_cs") }
-};
-static CRITICAL_SECTION handle_map_cs = { &handle_map_critsect_debug, -1, 0, 0, 0, 0 };
+DECLARE_CRITICAL_SECTION(handle_map_cs);
 
 NTSTATUS kernel_object_from_handle( HANDLE handle, POBJECT_TYPE type, void **ret )
 {
diff --git a/dlls/ntoskrnl.exe/ntoskrnl_private.h b/dlls/ntoskrnl.exe/ntoskrnl_private.h
index 295ff7397c..c338f08e15 100644
--- a/dlls/ntoskrnl.exe/ntoskrnl_private.h
+++ b/dlls/ntoskrnl.exe/ntoskrnl_private.h
@@ -21,6 +21,13 @@
 #ifndef __WINE_NTOSKRNL_PRIVATE_H
 #define __WINE_NTOSKRNL_PRIVATE_H
 
+#define DECLARE_CRITICAL_SECTION(cs) \
+    static CRITICAL_SECTION cs; \
+    static CRITICAL_SECTION_DEBUG cs##_debug = \
+    { 0, 0, &cs, { &cs##_debug.ProcessLocksList, &cs##_debug.ProcessLocksList }, \
+      0, 0, { (DWORD_PTR)(__FILE__ ": " # cs) }}; \
+    static CRITICAL_SECTION cs = { &cs##_debug, -1, 0, 0, 0, 0 };
+
 struct _OBJECT_TYPE
 {
     const WCHAR *name;            /* object type name used for type validation */

[Attachment #4 (text/plain)]




[prev in list] [next in list] [prev in thread] [next in thread]