Re: java.io.DataInputStream.readLine misbehaviour

[Mark Wielaard]

Hi,

On Fri, 2003-10-31 at 19:12, Guilhem Lavaux wrote:
> Continuing the Classpath-Kaffe merge, I noticed that 
> DataInputStream is failing one of kaffe's regression test 
> (InputStreamTest). It seems the failure comes from 
> readLine(): readLine() is a little too conservative compared 
> to JDK's implementation. The real behaviour seems to be the 
> following:
> 
> readLine() is invoked => it reads bytes until it finds a 
> '\r' => it reads the next byte
> * if it is  a '\n' it eats it
> * if it isn't it keeps it in an internal buffer until 
> someone calls another read methods.
>
> But in any case the byte isn't put back in the input buffer. 

The Classpath implementation clearly took the lowest overhead for the
non-readLine case. For the benefit of the other mailinglist readers,
here is the actual comment from Classpaths (same for libgcj)
DataInputStream.readLine():

    // FIXME: The following code tries to adjust the stream back one
    // character if the next char read is '\n'.  As a last resort,
    // it tries to mark the position before reading but the bottom
    // line is that it is possible that this method will not properly
    // deal with a '\r' '\n' combination thus not fulfilling the
    // DataInput contract for readLine.  It's not a particularly
    // safe approach threadwise since it is unsynchronized and
    // since it might mark an input stream behind the users back.
    // Along the same vein it could try the same thing for
    // ByteArrayInputStream and PushbackInputStream, but that is
    // probably overkill since this is deprecated & BufferedInputStream
    // is the most likely type of input stream.
    //
    // The alternative is to somehow push back the next byte if it
    // isn't a '\n' or to have the reading methods of this class
    // keep track of whether the last byte read was '\r' by readLine
    // and then skip the very next byte if it is '\n'.  Either way,
    // this would increase the complexity of the non-deprecated methods
    // and since it is undesirable to make non-deprecated methods
    // less efficient, the following seems like the most reasonable
    // approach.

BTW I don't think we have to worry about the unsynchronized thing since
streams are already not thread safe.

>   The real problem is what happens when someone calls one of 
> the inherited method from FilterInputStream which are not 
> overloaded (e.g. available).
>
> Kaffe's answer was to create a mini-buffer with default 
> access in FilterInputStream. It is obvious it is clumsy 
> because it slightly changes the API, but the only other 
> option would be to overload all read functions.

That is overhead for every FilterInputStream, that should be avoided.
But I agree that the Classpath way is not very clean.

Would a solution be to do the following in the constructor?

  public DataInputStream (InputStream in)
  {
    super (in.markSupported() ? in : new BufferedInputStream(in, 1));
  }

And then use the fact that in will always be a BufferedInputStream in
the readLine method? This does create a small overhead for
DataInputStream in case it isn't already wrapped around something that
buffers already, but it would be easy to make it always work correctly.

Another question is how often (ever?) does this (mixing readLine() with
other DataInputStream and/or FilterInputStream calls happen in real
programs. The regression test of kaffe looks contrived, it even uses
StringInputStream which is an ugly (and thankfully deprecated) class.

Cheers,

Mark

signature.asc
Description: This is a digitally signed message part