Summary: | [WINCE] Implement TCSpinLock | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Patrick R. Gansterer <paroga> | ||||||
Component: | Web Template Framework | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | aroben, commit-queue, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Patrick R. Gansterer
2010-07-07 12:59:16 PDT
Created attachment 60768 [details]
Patch
Attachment 60768 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
JavaScriptCore/wtf/TCSpinLock.h:198: "windows.h" already included at JavaScriptCore/wtf/TCSpinLock.h:55 [build/include] [4]
JavaScriptCore/wtf/TCSpinLock.h:200: TCMalloc_SlowLock is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
JavaScriptCore/wtf/TCSpinLock.h:205: Place brace on its own line for function definitions. [whitespace/braces] [4]
JavaScriptCore/wtf/TCSpinLock.h:210: Place brace on its own line for function definitions. [whitespace/braces] [4]
JavaScriptCore/wtf/TCSpinLock.h:214: Place brace on its own line for function definitions. [whitespace/braces] [4]
JavaScriptCore/wtf/TCSpinLock.h:215: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5]
JavaScriptCore/wtf/TCSpinLock.h:225: Place brace on its own line for function definitions. [whitespace/braces] [4]
JavaScriptCore/wtf/TCSpinLock.h:225: TCMalloc_SlowLock is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
JavaScriptCore/wtf/TCSpinLock.h:226: One space before end of line comments [whitespace/comments] [5]
Total errors found: 9 in 2 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 60768 [details] Patch > +#elif OS(WINDOWS) I don't understand how this code will be compiled for WinCE. The first branch of this #if/#elif chain is: #if (CPU(X86) || CPU(X86_64) || CPU(PPC)) && (COMPILER(GCC) || COMPILER(MSVC)) Will WinCE not match that condition? > + LONG m_lockword; I think m_lockword is supposed to be a volatile LONG. > +static void TCMalloc_SlowLock(LPLONG lockword) { > + Sleep(0); // Yield immediately since fast path failed > + while (true) { > + if (!InterlockedExchange(lockword, 1)) > + return; > + > + Sleep(2); > + } > +} This could be a little simpler: Sleep(0); while (InterlockedExchange(lockword, 1)) Sleep(2); Created attachment 62428 [details] Patch (In reply to comment #3) > I don't understand how this code will be compiled for WinCE. The first branch of this #if/#elif chain is: > > #if (CPU(X86) || CPU(X86_64) || CPU(PPC)) && (COMPILER(GCC) || COMPILER(MSVC)) > > Will WinCE not match that condition? CPU(ARM) > > + LONG m_lockword; > > I think m_lockword is supposed to be a volatile LONG. No, otherwise you get "convert parameter 1 from 'volatile LONG *' to 'LPLONG'" Attachment 62428 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
JavaScriptCore/wtf/TCSpinLock.h:198: "windows.h" already included at JavaScriptCore/wtf/TCSpinLock.h:55 [build/include] [4]
JavaScriptCore/wtf/TCSpinLock.h:200: TCMalloc_SlowLock is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
JavaScriptCore/wtf/TCSpinLock.h:205: Place brace on its own line for function definitions. [whitespace/braces] [4]
JavaScriptCore/wtf/TCSpinLock.h:210: Place brace on its own line for function definitions. [whitespace/braces] [4]
JavaScriptCore/wtf/TCSpinLock.h:214: Place brace on its own line for function definitions. [whitespace/braces] [4]
JavaScriptCore/wtf/TCSpinLock.h:215: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5]
JavaScriptCore/wtf/TCSpinLock.h:225: Place brace on its own line for function definitions. [whitespace/braces] [4]
JavaScriptCore/wtf/TCSpinLock.h:225: TCMalloc_SlowLock is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
JavaScriptCore/wtf/TCSpinLock.h:226: One space before end of line comments [whitespace/comments] [5]
Total errors found: 9 in 2 files
If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #4) > Created an attachment (id=62428) [details] > Patch > > (In reply to comment #3) > > I don't understand how this code will be compiled for WinCE. The first branch of this #if/#elif chain is: > > > > #if (CPU(X86) || CPU(X86_64) || CPU(PPC)) && (COMPILER(GCC) || COMPILER(MSVC)) > > > > Will WinCE not match that condition? > CPU(ARM) Ah! > > > + LONG m_lockword; > > > > I think m_lockword is supposed to be a volatile LONG. > > No, otherwise you get "convert parameter 1 from 'volatile LONG *' to 'LPLONG'" I guess InterlockedExchange has a different signature on WinCE. Comment on attachment 62428 [details] Patch Clearing flags on attachment: 62428 Committed r63981: <http://trac.webkit.org/changeset/63981> All reviewed patches have been landed. Closing bug. |