Seems OK to me, but I leave it to Alan. On Tue, Oct 7, 2014 at 1:08 AM, Ivan Gerasimov <ivan.gerasi...@oracle.com> wrote:
> Thanks Martin for the comments! > > On 06.10.2014 21:22, Martin Buchholz wrote: > > Thanks for tackling this difficult area! > > Alan is the most qualified to review, but I'll throw in some comments. > > The title of the bug in jira doesn't match the one in the webrev - they > should be the same. > > Do you mean the summary in the regression test? > I'll update the Jira bug's title, the thread subject and the summary to be > the same. > > I'm not sure, but it looks like getAppend is called even on non-Windows > platforms, but there should be no need? > > > There is one place where we need it: in FileChannelImpl.position(), if in > append mode, we should return the file size (see JDK-6526860). > As the append flag was moved from FileChannelImpl down to FileDescriptor, > we need a way to retrieve this information back, so getAppend() had to be > introduced. > This is applicable to both Windows and Unix. > > This code seems already to be covered by > java/nio/channels/FileChannel/AtomicAppend.java, so I'm not introducing a > new test for it. > > Sincerely yours, > Ivan > > On Mon, Oct 6, 2014 at 3:41 AM, Ivan Gerasimov <ivan.gerasi...@oracle.com > > wrote: > >> Hello everybody! >> >> The append mode is emulated on Windows, therefore we have to keep a flag >> indicating that. >> >> With the current implementation, the FileDescriptor does not know if the >> file were opened with O_APPEND, and the flag is maintained at the upper >> level. >> This can cause inconsistency, when the FileDescriptor is reused to >> construct a new FileOutputStream, as there is no information available >> about the append/non-append mode. >> >> Even though the solution is quite straight-forward: moving the flag from >> FileOutputStream, FileDispatcherImpl and FileChannelImpl to the lower >> level of FileDescriptor, it touches 20 files across the source-code base. >> >> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8023173 >> WEBREV: http://cr.openjdk.java.net/~igerasim/8023173/0/webrev/ >> >> With the fix, all the io/nio tests, including the new one, pass on all >> available platforms. >> >> Would you please help review this fix? >> >> Sincerely yours, >> Ivan >> > > >
