Bug 20302 - Wrong signbit implementation for solaris platform
Summary: Wrong signbit implementation for solaris platform
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-08-06 07:51 PDT by Alexey Ushakov
Modified: 2011-03-24 13:12 PDT (History)
4 users (show)

See Also:


Attachments
signbit patch for solaris platform (459 bytes, patch)
2008-08-06 07:54 PDT, Alexey Ushakov
no flags Details | Formatted Diff | Diff
Proposed patch which compiles correctly in qt-4.7.1 with webkit enabled on Solaris 10 with SS12 C++ compiler (1.09 KB, patch)
2011-03-24 09:48 PDT, Ben Taylor
darin: review+
darin: commit-queue+
Details | Formatted Diff | Diff
Updated patch which compiles correctly in qt-4.7.1 with webkit enabled on Solaris 10 with SS12 C++ compiler, fixed variable "num" to be "x" (1.08 KB, patch)
2011-03-24 10:12 PDT, Ben Taylor
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Ushakov 2008-08-06 07:51:20 PDT
Solaris platform has incomplete implementation for signbit finction in JavaScriptCore/wtf/MathExtras.h.

Here is correct implementation of this function:

inline bool signbit(double num) { return copysign(1.0, num) < 0; }
Comment 1 Alexey Ushakov 2008-08-06 07:54:33 PDT
Created attachment 22680 [details]
signbit patch for solaris platform
Comment 2 Ben Taylor 2011-03-24 09:48:48 PDT
Created attachment 86785 [details]
Proposed patch which compiles correctly in qt-4.7.1 with webkit enabled on Solaris 10 with SS12 C++ compiler

Add a correct patch that includes a ChangeLog and proper patch using the webkit tools. This was done against webkit head and compiles correctly on Solaris 10/SS12 against qt-4.7.1
Comment 3 Darin Adler 2011-03-24 09:53:43 PDT
Comment on attachment 86785 [details]
Proposed patch which compiles correctly in qt-4.7.1 with webkit enabled on Solaris 10 with SS12 C++ compiler

View in context: https://bugs.webkit.org/attachment.cgi?id=86785&action=review

> Source/JavaScriptCore/wtf/MathExtras.h:93
> -inline bool signbit(double x) { return x < 0.0; } // FIXME: Wrong for negative 0.
> +inline bool signbit(double num) { return copysign(1.0, num) < 0; }

I guess you copied the version from MSVC, which is why the variable name is now num. I would prefer that this file use "x" more consistently or use a word like "number" rather than "num".
Comment 4 Ben Taylor 2011-03-24 10:12:06 PDT
Created attachment 86789 [details]
Updated patch which compiles correctly in qt-4.7.1 with webkit enabled on Solaris 10 with SS12 C++ compiler, fixed variable "num" to be "x"

Updated patch per request
Comment 5 WebKit Commit Bot 2011-03-24 12:14:25 PDT
Comment on attachment 86789 [details]
Updated patch which compiles correctly in qt-4.7.1 with webkit enabled on Solaris 10 with SS12 C++ compiler, fixed variable "num" to be "x"

Clearing flags on attachment: 86789

Committed r81882: <http://trac.webkit.org/changeset/81882>
Comment 6 WebKit Commit Bot 2011-03-24 12:14:29 PDT
All reviewed patches have been landed.  Closing bug.
Comment 7 WebKit Review Bot 2011-03-24 13:12:05 PDT
http://trac.webkit.org/changeset/81882 might have broken Qt Linux Release
The following tests are not passing:
editing/pasteboard/5065605.html
editing/pasteboard/display-block-on-spans.html
editing/pasteboard/paste-text-011.html
editing/pasteboard/paste-text-at-tabspan-002.html