Give default value of O_NOFOLLOW if it is not defined

[Alan Bateman]

On 11/01/2012 09:00, Charles Lee wrote:
>
> Hi Alan,
>
> Sorry for the late reply. I found my self on a very long way to get 
> the popular machine which is not support O_NOFOLLOW :-)
>
> Please help to review the latest patch. It is at 
> http://cr.openjdk.java.net/~littlee/OJDK-226/webrev.01/ 
> <http://cr.openjdk.java.net/%7Elittlee/OJDK-226/webrev.01/>
>
> Thank you very much.
>
This looks quite good.

The only problem I see is in UnixChannelFactory L224-225 where you check 
if the file is a sym link. If CREATE is specified and the file doesn't 
previously exist then this code will end up throwing an exception. Two 
other points on this are (a) I also think NOFOLLOW_LINKS should be 
rejected earlier, and (b) since we are accessing the file system to 
check the file then I think IOException would be better than UOE. So for 
UnixChannelFactory I suggest the following:

@@ -86,13 +84,13 @@ class UnixChannelFactory {
                      }
                      continue;
                  }
-                if (option == LinkOption.NOFOLLOW_LINKS) {
+                if (option == LinkOption.NOFOLLOW_LINKS && 
supportsNoFollowLinks()) {
                      flags.noFollowLinks = true;
                      continue;
                  }
                  if (option == null)
                      throw new NullPointerException();
-               throw new UnsupportedOperationException();
+               throw new UnsupportedOperationException(option + " not 
supported");
              }
              return flags;
          }
@@ -220,6 +218,15 @@ class UnixChannelFactory {
          // follow links by default
          boolean followLinks = true;
          if (!flags.createNew && (flags.noFollowLinks || 
flags.deleteOnClose)) {
+            if (flags.deleteOnClose && !supportsNoFollowLinks()) {
+                try {
+                    if (UnixFileAttributes.get(path, 
false).isSymbolicLink())
+                        throw new UnixException("DELETE_ON_CLOSE 
specified and file is a symbolic link");
+                } catch (UnixException x) {
+                    if (!flags.create || x.errno() != ENOENT)
+                        throw x;
+                }
+            }
              followLinks = false;
              oflags |= O_NOFOLLOW;
          }

Otherwise I have only minor comments:

1. In UnixNativeDispatcher. supportsNoFollowLinks then it would be 
better to check if O_NOFOLLOW != 0 (just case it's not a positive value).

2. In genUnixConstants.c I think we should include a small comment to be 
consistent with AT_SYMLINK_NOFOLLOW.

3. UnixPath - really minor comment is that the braces aren't needed and 
best to be locally consistent.

-Alan.





-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/nio-dev/attachments/20120111/ee845004/attachment.html