[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