[webkit-reviews] review denied: [Bug 86194] [EFL] Rounding errors on 32-bit machines causes tests to fail : [Attachment 141651] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon May 14 08:13:40 PDT 2012


Martin Robinson <mrobinson at webkit.org> has denied Christophe Dumez
<christophe.dumez at intel.com>'s request for review:
Bug 86194: [EFL] Rounding errors on 32-bit machines causes tests to fail
https://bugs.webkit.org/show_bug.cgi?id=86194

Attachment 141651: Patch
https://bugs.webkit.org/attachment.cgi?id=141651&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=141651&action=review


Seems sane, though I think this could use a little cleanup.

> Tools/Scripts/webkitdirs.pm:333
> +	   } else {
> +	       # Fall back to output of `arch', if it is present.
> +	       $architecture = `arch`;
> +	       chomp $architecture;

The fallback is now repeated three times. Perhaps it's better to put it in a
helper function at this point or restructure the code so it's not repeated.

> Tools/Scripts/webkitdirs.pm:2100
> +    if ($architecture ne "x86_64" && !isARM()) {
> +	   $ENV{'CXXFLAGS'} = "-march=pentium4 -msse2 -mfpmath=sse";
> +    }

You're completely overwriting CXXFLAGS here. It might be better to preserve any
that already exist.


More information about the webkit-reviews mailing list