[webkit-reviews] review granted: [Bug 34380] [WINCE] Buildfix for ThreadSpecific.h : [Attachment 47776] The patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sun Jan 31 19:47:35 PST 2010
Darin Adler <darin at apple.com> has granted Patrick R. Gansterer
<paroga at paroga.com>'s request for review:
Bug 34380: [WINCE] Buildfix for ThreadSpecific.h
https://bugs.webkit.org/show_bug.cgi?id=34380
Attachment 47776: The patch
https://bugs.webkit.org/attachment.cgi?id=47776&action=review
------- Additional Comments from Darin Adler <darin at apple.com>
> +// TLS_OUT_OF_INDEXES is not defined on WinCE.
> +#ifndef TLS_OUT_OF_INDEXES
> +#define TLS_OUT_OF_INDEXES 0xffffffff
> +#endif
I guess this is OK, but I don't see any check for WinCE here. Might be slightly
better to explicitly use a WINCE ifdef so we’d get a warning instead of just
using the wrong value if someone made a mistake and did not get the
TLS_OUT_OF_INDEXES constant.
> - DWORD tls_key = TlsAlloc();
> - if (tls_key == TLS_OUT_OF_INDEXES)
> + DWORD tlsKey = TlsAlloc();
> + if (tlsKey == TLS_OUT_OF_INDEXES)
> CRASH();
>
> m_index = InterlockedIncrement(&tlsKeyCount()) - 1;
> if (m_index >= kMaxTlsKeySize)
> CRASH();
> - tlsKeys()[m_index] = tls_key;
> + tlsKeys()[m_index] = tlsKey;
The unrelated style change really does not belong on the same patch.
More information about the webkit-reviews
mailing list