harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Hindess <mark.hind...@googlemail.com>
Subject Re: [classlib] BufferedReader and FileInputStream.available()
Date Wed, 04 Nov 2009 08:21:55 GMT

In message <4AF0DC18.1000504@gmail.com>, Regis writes:
>
> enh wrote:
> > [apologies for the probably-busted formatting in this mail, but i
> > didn't actually receive the mail i'm replying to.]
> > 
> > Mark Hindess wrote:
> >> Our implementation of BufferedReader calls FileInputStream.available()
> >> while the RI does not.
> > 
> > well, it's InputStreamReader's fault, but "yes".
> > 
> >> This is a problem because it means that the RI
> >> can read certain types of files that our implementation can not - such
> >> as /proc/mounts on Linux.  I will investigate this shortly.
> > 
> > indeed. stat(2) will lie to you about many files in /proc too, which
> > confuses people who [ignoring the obvious race] try to allocate a
> > right-sized buffer before they start reading. the kernel really
> > assumes you're going to use the traditional Unix "repeatedly call
> > read(2) until it returns 0 at EOF" idiom. (which is fair enough, since
> > it doesn't know what data it's going to return until you open the
> > file.)
> > 
> >> While looking at this I notice that our implementation of
> >> FileInputStream.available() does:
> >>
> >>  long currentPosition = fileSystem.seek(fd.descriptor, 0L,
> >>                                         IFileSystem.SEEK_CUR);
> >>  long endOfFilePosition = fileSystem.seek(fd.descriptor, 0L,
> >>                                           IFileSystem.SEEK_END);
> >>  fileSystem.seek(fd.descriptor, currentPosition, IFileSystem.SEEK_SET);
> >>  return (int) (endOfFilePosition - currentPosition);
> >>
> >> making three JNI calls.  It might be better to implement this as a
> >> single JNI call - particularly since at the moment we seem to be calling
> >> it more often than the RI.
> > 
> > in Android -- where we have a Linux kernel and do want to be able to
> > read files in /proc -- we already went this route. we effectively have
> > an "available" in OSFileSystem. on Linux, we can use the FIONREAD
> > ioctl(2).
> > 
> > you already have this code lying around in harmony, but you only use
> > it for stdin and ProcessInputStream. grep for FIONREAD and work your
> > way back. (from my orthodox Unix perspective, distinguishing between
> > regular files and ttys in the Java side is a mistake. only the native
> > code for platforms that make such a distinction should pay for their
> > mistake.)
> > 
> > anyway, yeah, an intention-revealing "int OSFileSystem.available(int
> > fd)" for the Java side, and letting the native side worry about how
> > best to implement it sounds like a good idea.
> > 
> 
> It seems the same issue with HARMONY-6216. I was trying to fix it in
> java side but failed, implementing "available" in native side looks
> like a better way.

Regis, these are really two different issues so lets treat them as
such.  I actually think fixing the /proc reading is better done in
the java code.  I've attached a patch that just catches and ignores
the IOException from the available call.  I think this is reasonable
since if the exception was "real" then the read will fail if a similar
exception.  In fact, since the user is doing a read call, having the
exception thrown by the read is more in keeping with the expectations of
the user than having an exception thrown by the available call.

Making the available call (or part of it) a single native method call
is a separate issue.

Regards,
 Mark.



Mime
View raw message