Re: Patches for java.io.StreamTokenizer

[Tom Tromey <tromey at cygnus dot com>]

>>>>> "Jeff" == Jeff Sturm <jsturm@sigma6.com> writes:

Jeff> The patch below prevents an infinite loop in StreamTokenizer.
Jeff> (Attempting to unread(-1) on a PushbackReader causes the next
Jeff> read to return 65535.  I thought this was a bug in
Jeff> PushbackReader, but other implementations seem to do the same
Jeff> thing.  I guess the consequence of unread(-1) is simply
Jeff> undefined.)

The JCL book says that PushbackReader.unread() pushes back the
low-order 2 bytes of the argument.  The online JDK 1.2 docs say
nothing.

Jeff> There are other problems in StreamTokenizer that probably can't
Jeff> be fixed by a small patch.  For example, "-." is incorrectly
Jeff> read as a number, causing NumberFormatException.  IMHO the body
Jeff> of nextToken() needs to be reimplemented as a FSM.  Would it be
Jeff> worthwhile to rewrite this class, or can we consider getting it
Jeff> from the Classpath project instead?

The current plan is to choose between libgcj and Classpath classes on
a case-by-case basis -- assuming RMS gives his ok to the whole process
(I don't have a clue why it is taking so long).

What I'd really like to see are Mauve tests that we can use to help
make the determination.

My only problems with this patch are coding style bugs:

Jeff>  	  if (ch == '\r' && (ch = in.read()) != '\n')
Jeff> -	    in.unread(ch);
Jeff> +	    if (ch != TT_EOF) in.unread(ch);

We don't put the body on the same line as the "if" (there are a few of
these in the patch).  Also, nested if's required braces.

Jeff>  		    else
Jeff> -		      in.unread(ch);
Jeff> +		      if (ch != TT_EOF) in.unread(ch);

Here the "if" should be on the same line as the "else".

Jeff>  		      if (ch == '\r' && (ch = in.read()) != '\n')
Jeff> -			in.unread(ch);
Jeff> +		        if (ch != TT_EOF) in.unread(ch);

Here you should just add another "&&" clause to the "if".
(Warning in advance: the "&&" must come after a line break, not
before.)

If you make these changes, I'll check this in.

Thanks,
Tom