Re: 6988037: fileClose prints debug message if close fails
Hi Alan,
That sounds familiar and looks good. (There has been some testing with
such a change already 8-) )
Regards
Kevin
On 29/09/2010 13:25, Alan Bateman wrote:
I need a reviewer for a small change to remove a debug message that
someone left when fixing a bug in the java.io implementation early in
jdk7. The debug message is printed if closing a file fails (for
example EIO because of a NFS stale handle). That change also added
code to restore the file descriptor field but this isn't needed
because the stream classes have a closed flag to ensure that they only
attempt to close the underlying file descriptor once. Furthermore, if
close fails then the state of the file descriptor is unspecified and
so cannot be used again. The proposed patch is attached.
Thanks,
Alan.
diff --git a/src/solaris/native/java/io/io_util_md.c
b/src/solaris/native/java/io/io_util_md.c
--- a/src/solaris/native/java/io/io_util_md.c
+++ b/src/solaris/native/java/io/io_util_md.c
@@ -83,8 +83,6 @@ fileClose(JNIEnv *env, jobject this, jfi
close(devnull);
}
} else if (JVM_Close(fd) == -1) {
-SET_FD(this, fd, fid); // restore fd
-printf(JVM_Close returned -1\n);
-JNU_ThrowIOExceptionWithLastError(env, close failed);
+JNU_ThrowIOExceptionWithLastError(env, close failed);
}
}
diff --git a/src/windows/native/java/io/io_util_md.c
b/src/windows/native/java/io/io_util_md.c
--- a/src/windows/native/java/io/io_util_md.c
+++ b/src/windows/native/java/io/io_util_md.c
@@ -531,7 +531,6 @@ handleClose(JNIEnv *env, jobject this, j
SET_FD(this, -1, fid);
if (CloseHandle(h) == 0) { /* Returns zero on failure */
-SET_FD(this, fd, fid); // restore fd
JNU_ThrowIOExceptionWithLastError(env, close failed);
}
return 0;
Re: 6728842: File.setReadOnly does not make a directory read-only (win)
Le 29/09/2010 15:57, Alan Bateman a écrit : I need a reviewer for a change to java.io.File's setReadOnly and setWritable methods so that they don't change the DOS readonly attribute on directories (or folders as Windows calls them). These methods have never worked correctly for directories on Windows because the semantics of this attribute is different for directories (it determines if the directory can be deleted, not whether is is writable, and is used by Windows Explorer to determine if the directory is special, sending it off looking for a hidden Desktop.ini file). Early on in jdk7, the canWrite method was changed to ignore the readonly attribute on directories so changing the set* methods to ignore the attribute restores the status quo. I should say that changing the implementation has some risk. If it does cause problems they we may need to include a compatibility switch. The alternative to ignoring the readonly attribute is to insert a deny rule into the directory's DACL (when on NTFS) but that has a lot of potential to cause problems for end-users. As part of the change proposed here, the SetAccess test has been changed so that it no longer execs ls -l and this fixes another bug (6464744) where the test fails if the sticky bit is set. The webrev with the changes is here: http://cr.openjdk.java.net/~alanb/6728842/webrev/ Thanks, Alan. There is a small copy/paste error in setAccess.doTest() for Windows specific tests, the exception message of the thrown Exception doesn't match the test. // setWritable should fail on directories because the DOS readonly 148 // attribute prevents a directory from being deleted. 149 if (f.setWritable(false, true)) 150 throw new Exception(f + : setWritable(false, true) Succeeded); 151 if (f.setWritable(false, false)) 152 throw new Exception(f + : setWritable(false, true) Succeeded); 153 if (f.setWritable(false)) 154 throw new Exception(f + : setWritable(false, true) Succeeded); should be // setWritable should fail on directories because the DOS readonly 148 // attribute prevents a directory from being deleted. 149 if (f.setWritable(false, true)) 150 throw new Exception(f + : setWritable(false, true) Succeeded); 151 if (f.setWritable(false, false)) 152 throw new Exception(f + : setWritable(false, false) Succeeded); 153 if (f.setWritable(false)) 154 throw new Exception(f + : setWritable(false) Succeeded); Other changes look fine. Rémi
Re: 6728842: File.setReadOnly does not make a directory read-only (win)
Rémi Forax wrote: : There is a small copy/paste error in setAccess.doTest() for Windows specific tests, the exception message of the thrown Exception doesn't match the test. Well spotted! I'll fix up these messages in the test before I push this. -Alan.
hg: jdk7/tl/langtools: 6502392: Invalid relative names for Filer.createResource and Filer.getResource
Changeset: f94af0667151 Author:jjg Date: 2010-09-29 14:01 -0700 URL: http://hg.openjdk.java.net/jdk7/tl/langtools/rev/f94af0667151 6502392: Invalid relative names for Filer.createResource and Filer.getResource Reviewed-by: darcy ! src/share/classes/com/sun/tools/javac/file/JavacFileManager.java + test/tools/javac/processing/filer/TestInvalidRelativeNames.java
hg: jdk7/tl/jdk: 6988163: sun.security.util.Resources dup and a keytool doc typo
Changeset: 26c6ee936f63 Author:weijun Date: 2010-09-29 15:26 +0800 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/26c6ee936f63 6988163: sun.security.util.Resources dup and a keytool doc typo Reviewed-by: xuelei ! src/share/classes/sun/security/tools/KeyTool.java ! src/share/classes/sun/security/util/Resources.java
6988037: fileClose prints debug message if close fails
I need a reviewer for a small change to remove a debug message that
someone left when fixing a bug in the java.io implementation early in
jdk7. The debug message is printed if closing a file fails (for example
EIO because of a NFS stale handle). That change also added code to
restore the file descriptor field but this isn't needed because the
stream classes have a closed flag to ensure that they only attempt to
close the underlying file descriptor once. Furthermore, if close fails
then the state of the file descriptor is unspecified and so cannot be
used again. The proposed patch is attached.
Thanks,
Alan.
diff --git a/src/solaris/native/java/io/io_util_md.c
b/src/solaris/native/java/io/io_util_md.c
--- a/src/solaris/native/java/io/io_util_md.c
+++ b/src/solaris/native/java/io/io_util_md.c
@@ -83,8 +83,6 @@ fileClose(JNIEnv *env, jobject this, jfi
close(devnull);
}
} else if (JVM_Close(fd) == -1) {
-SET_FD(this, fd, fid); // restore fd
-printf(JVM_Close returned -1\n);
-JNU_ThrowIOExceptionWithLastError(env, close failed);
+JNU_ThrowIOExceptionWithLastError(env, close failed);
}
}
diff --git a/src/windows/native/java/io/io_util_md.c
b/src/windows/native/java/io/io_util_md.c
--- a/src/windows/native/java/io/io_util_md.c
+++ b/src/windows/native/java/io/io_util_md.c
@@ -531,7 +531,6 @@ handleClose(JNIEnv *env, jobject this, j
SET_FD(this, -1, fid);
if (CloseHandle(h) == 0) { /* Returns zero on failure */
-SET_FD(this, fd, fid); // restore fd
JNU_ThrowIOExceptionWithLastError(env, close failed);
}
return 0;
Re: 6988037: fileClose prints debug message if close fails
Looks good.
Rémi
Le 29/09/2010 14:25, Alan Bateman a écrit :
I need a reviewer for a small change to remove a debug message that
someone left when fixing a bug in the java.io implementation early in
jdk7. The debug message is printed if closing a file fails (for
example EIO because of a NFS stale handle). That change also added
code to restore the file descriptor field but this isn't needed
because the stream classes have a closed flag to ensure that they only
attempt to close the underlying file descriptor once. Furthermore, if
close fails then the state of the file descriptor is unspecified and
so cannot be used again. The proposed patch is attached.
Thanks,
Alan.
diff --git a/src/solaris/native/java/io/io_util_md.c
b/src/solaris/native/java/io/io_util_md.c
--- a/src/solaris/native/java/io/io_util_md.c
+++ b/src/solaris/native/java/io/io_util_md.c
@@ -83,8 +83,6 @@ fileClose(JNIEnv *env, jobject this, jfi
close(devnull);
}
} else if (JVM_Close(fd) == -1) {
-SET_FD(this, fd, fid); // restore fd
-printf(JVM_Close returned -1\n);
-JNU_ThrowIOExceptionWithLastError(env, close failed);
+JNU_ThrowIOExceptionWithLastError(env, close failed);
}
}
diff --git a/src/windows/native/java/io/io_util_md.c
b/src/windows/native/java/io/io_util_md.c
--- a/src/windows/native/java/io/io_util_md.c
+++ b/src/windows/native/java/io/io_util_md.c
@@ -531,7 +531,6 @@ handleClose(JNIEnv *env, jobject this, j
SET_FD(this, -1, fid);
if (CloseHandle(h) == 0) { /* Returns zero on failure */
-SET_FD(this, fd, fid); // restore fd
JNU_ThrowIOExceptionWithLastError(env, close failed);
}
return 0;
6728842: File.setReadOnly does not make a directory read-only (win)
I need a reviewer for a change to java.io.File's setReadOnly and setWritable methods so that they don't change the DOS readonly attribute on directories (or folders as Windows calls them). These methods have never worked correctly for directories on Windows because the semantics of this attribute is different for directories (it determines if the directory can be deleted, not whether is is writable, and is used by Windows Explorer to determine if the directory is special, sending it off looking for a hidden Desktop.ini file). Early on in jdk7, the canWrite method was changed to ignore the readonly attribute on directories so changing the set* methods to ignore the attribute restores the status quo. I should say that changing the implementation has some risk. If it does cause problems they we may need to include a compatibility switch. The alternative to ignoring the readonly attribute is to insert a deny rule into the directory's DACL (when on NTFS) but that has a lot of potential to cause problems for end-users. As part of the change proposed here, the SetAccess test has been changed so that it no longer execs ls -l and this fixes another bug (6464744) where the test fails if the sticky bit is set. The webrev with the changes is here: http://cr.openjdk.java.net/~alanb/6728842/webrev/ Thanks, Alan.
