Re: [cp-patches] Re: FYI: java/io/FileInputStream.java& java/io/FileOutputStream.java

[Dalibor Topic]

Jeroen Frijters wrote:
> Dalibor Topic wrote:
>
> > Jeroen Frijters <jeroen <at> sumatra.nl> writes:
> >
> > > 2005-03-03  Jeroen Frijters  <jeroen <at> frijters.net>
> > >
> > >        * java/io/FileInputStream.java (FileInputStream(File)),
> > >        java/io/FileOutputStream.java (FileOutputStream(File)):
> > >        Removed unnecessary File.isDirectory() check.
> >
> > Hi Jeroen,
> >
> > while trying to merge the patch into Kaffe I noticed that it breaks 
> > the http://www.kaffe.org/cgi-bin/viewcvs.cgi/kaffe/test/regression/
> > FileChecks.java?rev=1.2&content-type=text/vnd.viewcvs-markup
> > regression test in Kaffe. According to API docs for File*outStream,
> > the File constructor is supposed to throw an exception if the file
> > parameter  is a directory.
> >
> > Could you elaborate some more on the patch?
>
>
> This means that FileChannelImpl.open() should be fixed to throw the
> proper exception. Doing the explicit check in the File[In|Out]putStream
> doesn't make sense, because it is inherently racy (and inefficient too).

Yeah, i can understand the inefficient part, as the same code is 
duplicated in two classes. Could you elaborate some more on the racy part?

I've been working on a patch that moves the check into the constructor 
of FileChannelImpl, which is, afaik, the only code that calls open(). It 
passes the kaffe regression tests for me, and now I'm running mauve over 
it, to see if I broke something accidentally.

> I didn't notice the failure because I use a custom version of
> FileChannelImpl, so sorry about that.

No worries, that's what hand merging is good for: time-shifted peer 
review :)

cheers,
dalibor topic