[patch] "\r\n" handling in DataInputStream.readLine()

[Bryce McKinlay]

Per Bothner wrote:

> I have mixed feelings about this situation.
>
> The correct solution is to use a flag indicating if the previous byte was
> a '\r'.  However, if we are going to do that, we might as well do it
> correctly, and change the various read methods to check the flag.  (We
> can do some minor optimization by replacing read() by in.read().)
>
> However, I think it is quite reasonable to leave things as is.  The
> readLine() method is inherently bogus in its design - which is why it has
> been deprectated.  DataInputStream is for *binary* I/O.  A method like
> readLine make no sense in that context:  You can't fiddle with unknown EOL
> conventions in the middle of a file of binary data.  People should not
> be using it.  (If they do use it, they should be using a BufferedInputStream.)

I agree that its not worth sacrificing extra complexity in other methods to 100%
correctly implement a broken (and deprecated) spec.

My patch (I believe) correctly implements readLine() assuming it is not called in
combination with the other read methods. I think it is very unlikely that code
that is using the readLine() method is going to be calling combinations of
readLine() and the other read() methods on the same stream.

In any case, the behaviour with the patch is better than how it is implemented
now.

> Note if we leave the code as is, I think there is still a bug:  The tests
> for  (read() & 0xFF) != '\n' will do the wrong thing on EOF.

Do you mean the existing "instanceof BufferedInputStream" hack? So doing a pos--
on a BufferedInputStream will cause the same \r to be re-read in the case where
EOF follows a '\r'. ? If so a check needs to be added. Or am I missing something
else?

regards

  [ bryce ]