[Bug 25108] New: Critical sections used without calling the initialization function
http://bugs.winehq.org/show_bug.cgi?id=25108 Summary: Critical sections used without calling the initialization function Product: Wine Version: 1.3.6 Platform: x86 OS/Version: Linux Status: UNCONFIRMED Severity: blocker Priority: P2 Component: ntdll AssignedTo: wine-bugs(a)winehq.org ReportedBy: 8jmtfa1e(a)gmail.com I was doing some experiments with dlls/ntdll/critsection.c trying to verify if it was causing the huge performance penalty in Starcraft II. My experiments stalled when I realized that some parts of wine used critical sections without even bother calling RtlInitializeCriticalSection first. That's an abomination and a great example of spaghetti code. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=25108 Nikolay Sivov <bunglehead(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Component|ntdll |-unknown Severity|blocker |normal --- Comment #1 from Nikolay Sivov <bunglehead(a)gmail.com> 2010-11-10 17:31:35 CST --- What parts of Wine? Please be more specific. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=25108 Dmitry Timoshkov <dmitry(a)codeweavers.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |RESOLVED Resolution| |INVALID --- Comment #2 from Dmitry Timoshkov <dmitry(a)codeweavers.com> 2010-11-11 04:14:18 CST --- Critical sections can be statically initialized instead of calling InitializeCriticalSection(), that's perfectly acceptable. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=25108 Dmitry Timoshkov <dmitry(a)codeweavers.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #3 from Dmitry Timoshkov <dmitry(a)codeweavers.com> 2010-11-11 04:14:41 CST --- Closing invalid. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=25108 --- Comment #4 from Eric <8jmtfa1e(a)gmail.com> 2010-11-11 06:36:44 CST --- (In reply to comment #2)
Critical sections can be statically initialized instead of calling InitializeCriticalSection(), that's perfectly acceptable.
I would call it undefined behaviour. No one outside of critsection.c should know it's internal configurations. That's why there's an initializations function. The purpose of the function is to make sure that internal changes should not effect other parts of the project. Initializing the structure elsewhere is insane and it's not considered a good coding practice. Even Microsoft agrees with me. Reading from this document http://msdn.microsoft.com/en-us/magazine/cc164040.aspx you'll find this quote "The life of a critical section begins when it's passed to InitializeCriticalSection (or more accurately, when its address is passed). Once initialized, your code passes the critical section to the EnterCriticalSection and LeaveCriticalSection APIs." Notice that they never mentions statically initialized structures. That's because a programmer would never consider doing such a thing unless there's absolutely no other way to solve the problem. It's one of the basic principles of objective oriented programming. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=25108 --- Comment #5 from Dmitry Timoshkov <dmitry(a)codeweavers.com> 2010-11-11 06:57:36 CST --- (In reply to comment #4)
(In reply to comment #2)
Critical sections can be statically initialized instead of calling InitializeCriticalSection(), that's perfectly acceptable.
I would call it undefined behaviour. No one outside of critsection.c should know it's internal configurations.
I'd say that's up to an implementor. Especially since in the case of Wine code the implementor and user of the API is the same "person".
Even Microsoft agrees with me.
That's the documentation for the application programmers, not for the API implementors. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=25108 Vladimir Voznesensky <vvoznesensky(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |vvoznesensky(a)gmail.com --- Comment #6 from Vladimir Voznesensky <vvoznesensky(a)gmail.com> 2011-03-21 03:56:04 CDT --- You may be interested in http://bugs.winehq.org/show_bug.cgi?id=26500 -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
participants (1)
-
wine-bugs＠winehq.org