Fixed infinite loops and buffer overflow in byte_scan when scanning for integ...

Fixed infinite loops and buffer overflow in byte_scan when scanning for integers and floating-point numbers.
- The byte_scan and associated functions are not very well written. I tried to clean them as much as possible without re-writing the whole thing.
- Replaced lBuildFloatValue function with atof. lBuildFloatValue was returning incorrect value anyway. The only reason it was working so far because we never used that value.
BUG=59623(crbug.com), 603333(bugzilla.mozilla.org)

[dgkoch, 2010-10-25 17:57:55]

a couple of possible issues noted below. 

You've also ran the CTS on with this change?

Daniel

http://codereview.appspot.com/2655042/diff/1/src/compiler/preprocessor/scanner.c
File src/compiler/preprocessor/scanner.c (right):

http://codereview.appspot.com/2655042/diff/1/src/compiler/preprocessor/scanne...
src/compiler/preprocessor/scanner.c:247: yylvalpp->sc_fval = (float)
atof(yylvalpp->symbol_name);
we're trying to get rid of atof since it is locale dependent, but we'll just
have to pick this one up too when those changes get in (see Issue #32)

http://codereview.appspot.com/2655042/diff/1/src/compiler/preprocessor/scanne...
src/compiler/preprocessor/scanner.c:374: ch =
cpp->currentInput->getch(cpp->currentInput, yylvalpp);
aren't we losing the ch != '0' check here?

[Alok Priyadarshi, 2010-10-25 18:22:57]

Yes - CTS passes.

http://codereview.appspot.com/2655042/diff/1/src/compiler/preprocessor/scanner.c
File src/compiler/preprocessor/scanner.c (right):

http://codereview.appspot.com/2655042/diff/1/src/compiler/preprocessor/scanne...
src/compiler/preprocessor/scanner.c:247: yylvalpp->sc_fval = (float)
atof(yylvalpp->symbol_name);
On 2010/10/25 17:57:56, dgkoch wrote:
> we're trying to get rid of atof since it is locale dependent, but we'll just
> have to pick this one up too when those changes get in (see Issue #32)

OK.

http://codereview.appspot.com/2655042/diff/1/src/compiler/preprocessor/scanne...
src/compiler/preprocessor/scanner.c:374: ch =
cpp->currentInput->getch(cpp->currentInput, yylvalpp);
On 2010/10/25 17:57:56, dgkoch wrote:
> aren't we losing the ch != '0' check here?

Yes that is intentional. I think the original code was trying to prevent copying
'0' twice coming from the fall-through. If you look closely at case '0', you
will notice that in the case that falls through, '0' is never copied to
symbol_name.

[dgkoch, 2010-10-25 18:25:42]

OK. LGTM then.

[Alok Priyadarshi, 2010-10-25 19:36:10]

On 2010/10/25 18:25:42, dgkoch wrote:
> OK. LGTM then.

Ken: If you are not too busy, I would appreciate another set of eyes.

[kbr1, 2010-10-26 01:51:23]

LGTM though I didn't have time to go through it very carefully. One comment.

http://codereview.appspot.com/2655042/diff/1/src/compiler/preprocessor/scanner.c
File src/compiler/preprocessor/scanner.c (right):

http://codereview.appspot.com/2655042/diff/1/src/compiler/preprocessor/scanne...
src/compiler/preprocessor/scanner.c:248: if (isinff(yylvalpp->sc_fval)) {
Is isinff available on all platforms? Windows in particular?

[Alok Priyadarshi, 2010-10-26 04:29:07]

http://codereview.appspot.com/2655042/diff/1/src/compiler/preprocessor/scanner.c
File src/compiler/preprocessor/scanner.c (right):

http://codereview.appspot.com/2655042/diff/1/src/compiler/preprocessor/scanne...
src/compiler/preprocessor/scanner.c:248: if (isinff(yylvalpp->sc_fval)) {
On 2010/10/26 01:51:23, kbr1 wrote:
> Is isinff available on all platforms? Windows in particular?

isinff is defined in this file.

[kbr1, 2010-10-26 20:46:07]

On 2010/10/26 04:29:07, Alok Priyadarshi wrote:
>
http://codereview.appspot.com/2655042/diff/1/src/compiler/preprocessor/scanner.c
> File src/compiler/preprocessor/scanner.c (right):
> 
>
http://codereview.appspot.com/2655042/diff/1/src/compiler/preprocessor/scanne...
> src/compiler/preprocessor/scanner.c:248: if (isinff(yylvalpp->sc_fval)) {
> On 2010/10/26 01:51:23, kbr1 wrote:
> > Is isinff available on all platforms? Windows in particular?
> 
> isinff is defined in this file.

OK. LGTM