[webkit-reviews] review denied: [Bug 15416] dtoa Falls into infinite loop on ARMEL : [Attachment 19444] fix mixed-endian

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Feb 29 03:21:16 PST 2008


David Kilzer (ddkilzer) <ddkilzer at webkit.org> has denied 's request for review:
Bug 15416: dtoa Falls into infinite loop on ARMEL
http://bugs.webkit.org/show_bug.cgi?id=15416

Attachment 19444: fix mixed-endian
http://bugs.webkit.org/attachment.cgi?id=19444&action=edit

------- Additional Comments from David Kilzer (ddkilzer) <ddkilzer at webkit.org>
Please set the "review?" flag, not the "review+" flag, to have your patch
reviewed.

>+2008-02-28  weihongzeng  <set EMAIL_ADDRESS environment variable>

Please set your first/last name (not just your account) and your email address
in the ChangeLog file.

>+	  Reviewed by NOBODY (OOPS!).
>+
>+	  * kjs/dtoa.cpp:
>+	  (Bigint::):
>+	  (Bigint::freedtoa):

Please refer to this bug in the ChangeLog entry, and describe what you fixed. 
(See other ChangeLog entries for examples.)

>+#ifdef USE_LOCALE
>+	s1 = localeconv()->decimal_point;
>+	if (c == *s1) {
>+		c = '.';
>+		if (*++s1) {
>+			s2 = s;
>+			for(;;) {
>+				if (*++s2 != *s1) {
>+					c = 0;
>+					break;
>+					}
>+				if (!*++s1) {
>+					s = s2;
>+					break;
>+					}
>+				}
>+			}
>+		}
>+#endif

This is incorrect style.  Curly braces appear below their corresponding if
statement.  See this page for more information:

http://webkit.org/coding/coding-style.html

>-						while (*--s == '0') { }
>+						while(*--s == '0');

>-		while (*--s == '0') { }
>+		while(*--s == '0');

Why were these changes made?  I recall that some compilers require the empty
block instead of a semi-colon.	These seem unnecessary.

r- for code style issues and incomplete ChangeLog.


More information about the webkit-reviews mailing list