[webkit-reviews] review denied: [Bug 106740] Fix the atomicIncrement implementation for Windows : [Attachment 184205] patch for landing

[bugzilla-daemon at webkit.org]

Jocelyn Turcotte <jocelyn.turcotte at digia.com> has denied Zoltan Arvai
<zarvai at inf.u-szeged.hu>'s request for review:
Bug 106740: Fix the atomicIncrement implementation for Windows
https://bugs.webkit.org/show_bug.cgi?id=106740

Attachment 184205: patch for landing
https://bugs.webkit.org/attachment.cgi?id=184205&action=review

------- Additional Comments from Jocelyn Turcotte <jocelyn.turcotte at digia.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=184205&action=review


> Source/JavaScriptCore/ChangeLog:5
> +	   Dropping support for Windows versions before XP SP2
> +	   for fixing atomicIncrement implementation for Windows.
> +	   https://bugs.webkit.org/show_bug.cgi?id=106740

The subject line should be short enough to fit on one line.

> Source/JavaScriptCore/ChangeLog:11
> +	   Current WebKit configured to support Windows 2000 and later
versions.
> +	   After r139514 WebKit2 can't be build, because the required
InterlockedIncrement64
> +	   method support is available on XP SP2 and later. So we have to move
on.

The commit log editor will put all the ChangeLog bodies in the commit log. So
since they are the same it would be better to omit the body except for WebKit2
(where it matters here) to avoid it appearing 6 times.

> Source/WTF/ChangeLog:11
> +	   Current WebKit configured to support Windows 2000 and later
versions.
> +	   After r139514 WebKit2 can't be build, because the required
InterlockedIncrement64
> +	   method support is available on XP SP2 and later. So we have to move
on.

This should rather describe the change to Atomics.h, the separation of WinCE,
etc.

> Source/WTF/wtf/Atomics.h:87
> +inline int64_t atomicIncrement(int64_t* addend) { return
InterlockedIncrement64(reinterpret_cast<long long* >(addend)); }
> +inline int64_t atomicDecrement(int64_t* addend) { return
InterlockedDecrement64(reinterpret_cast<long long* >(addend)); }

Unneeded extra space before the last ">".