Skip to content

8365807: (fs) Two-arg UnixFileAttributes.getIfExists should not use exception for control flow #26889

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+16 −38
Open

8365807: (fs) Two-arg UnixFileAttributes.getIfExists should not use exception for control flow #26889

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a4545b9
8365807: (fs) Two-arg UnixFileAttributes.getIfExists should not use e…
bplb Aug 21, 2025
7ea1b08
8365807: Make all [f]stat*0 functions void; remove stat2
bplb Aug 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 6 additions & 16 deletions src/java.base/unix/classes/sun/nio/fs/UnixFileAttributes.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,18 +83,9 @@ static UnixFileAttributes get(UnixPath path, boolean followLinks)

// get the UnixFileAttributes for a given file.
// Returns null if the file does not exist.
static UnixFileAttributes getIfExists(UnixPath path)
throws UnixException
static UnixFileAttributes getIfExists(UnixPath path) throws UnixException
{
UnixFileAttributes attrs = new UnixFileAttributes();
int errno = UnixNativeDispatcher.stat2(path, attrs);
if (errno == 0) {
return attrs;
} else if (errno == UnixConstants.ENOENT) {
return null;
} else {
throw new UnixException(errno);
}
return getIfExists(path, true);
}

// get the UnixFileAttributes for a given file, optionally following links.
Expand All @@ -106,14 +97,13 @@ static UnixFileAttributes getIfExists(UnixPath path, boolean followLinks)
int flag = (followLinks) ? 0 : UnixConstants.AT_SYMLINK_NOFOLLOW;
try {
UnixNativeDispatcher.fstatat(UnixConstants.AT_FDCWD,
path.asByteArray(), flag, attrs);
path, flag, attrs);
} catch (UnixException x) {
if (x.errno() == UnixConstants.ENOENT)
return null;

throw x;
else
throw x;
}

return attrs;
}

Expand All @@ -130,7 +120,7 @@ static UnixFileAttributes get(int dfd, UnixPath path, boolean followLinks)
{
UnixFileAttributes attrs = new UnixFileAttributes();
int flag = (followLinks) ? 0 : UnixConstants.AT_SYMLINK_NOFOLLOW;
UnixNativeDispatcher.fstatat(dfd, path.asByteArray(), flag, attrs);
UnixNativeDispatcher.fstatat(dfd, path, flag, attrs);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If dfd/path does not exist then won't this now return an unpopulated UnixFileAttributes?

I think my concern is that the changes create a bit of a hazard. Most UnixNativeDispatcher native methods throw if the syscall fails, a small number return the errno, now we have a native method that special cases ENOENT. I think we should try to limit it to two variants, one that throws, the other that returns errno, and establish some naming convention to reduce the possibility of mis-use. It would be good to audit out tests to make sure that every method exercises error cases.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change was intended to address this comment

a version that doesn't use exceptions for control flow,

but I think I misinterpreted that. I'll reassess the change.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit 7ea1b08:

  1. Change fstatat0 and stat0 to throw instead of returning errno.
  2. Remove the stat2 method.
  3. Change calling code to handle the changes.

Now all ?stat{at}0 functions are void, expressing errors via UnixException.

All UnixNativeDispatcher methods are now void except:

jbytearray getcwd, getgrid, getgrnam, getpwnam, getpwuid, readdir, realpath, strerror
jint       access, dup, fgetxattr, flistxattr, getlinelen, openat, read, write
jlong      fdopendir, opendir

All these functions return a "useful" value, except access which returns errno. It would be a little disruptive to change access to throw instead of returning errno as there are a number of usages like

return access(path, mode) == 0;

return attrs;
}

Expand Down
18 changes: 4 additions & 14 deletions src/java.base/unix/classes/sun/nio/fs/UnixNativeDispatcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,20 +250,10 @@ private static native void symlink0(long name1, long name2)
*/
static void stat(UnixPath path, UnixFileAttributes attrs) throws UnixException {
try (NativeBuffer buffer = copyToNativeBuffer(path)) {
int errno = stat0(buffer.address(), attrs);
if (errno != 0) {
throw new UnixException(errno);
}
}
}

static int stat2(UnixPath path, UnixFileAttributes attrs) {
try (NativeBuffer buffer = copyToNativeBuffer(path)) {
return stat0(buffer.address(), attrs);
stat0(buffer.address(), attrs);
}
}

private static native int stat0(long pathAddress, UnixFileAttributes attrs);
private static native void stat0(long pathAddress, UnixFileAttributes attrs);

/**
* lstat(const char* path, struct stat* buf)
Expand All @@ -288,10 +278,10 @@ private static native void fstat0(int fd, UnixFileAttributes attrs)
/**
* fstatat(int filedes,const char* path, struct stat* buf, int flag)
*/
static void fstatat(int dfd, byte[] path, int flag, UnixFileAttributes attrs)
static void fstatat(int dfd, UnixPath path, int flag, UnixFileAttributes attrs)
throws UnixException
{
try (NativeBuffer buffer = NativeBuffers.asNativeBuffer(path)) {
try (NativeBuffer buffer = copyToNativeBuffer(path)) {
fstatat0(dfd, buffer.address(), flag, attrs);
}
}
Expand Down
14 changes: 6 additions & 8 deletions src/java.base/unix/native/libnio/fs/UnixNativeDispatcher.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,7 @@ static void copy_stat_attributes(JNIEnv* env, struct stat* buf, jobject attrs) {
#endif
}

JNIEXPORT jint JNICALL
JNIEXPORT void JNICALL
Java_sun_nio_fs_UnixNativeDispatcher_stat0(JNIEnv* env, jclass this,
jlong pathAddress, jobject attrs)
{
Expand All @@ -662,18 +662,16 @@ Java_sun_nio_fs_UnixNativeDispatcher_stat0(JNIEnv* env, jclass this,
RESTARTABLE(statx_wrapper(AT_FDCWD, path, flags, mask, &statx_buf), err);
if (err == 0) {
copy_statx_attributes(env, &statx_buf, attrs);
return 0;
} else {
return errno;
throwUnixException(env, errno);
}
}
#endif
RESTARTABLE(stat(path, &buf), err);
if (err == 0) {
copy_stat_attributes(env, &buf, attrs);
return 0;
} else {
return errno;
throwUnixException(env, errno);
}
}

Expand Down Expand Up @@ -774,10 +772,10 @@ Java_sun_nio_fs_UnixNativeDispatcher_fstatat0(JNIEnv* env, jclass this, jint dfd
return;
}
RESTARTABLE((*my_fstatat_func)((int)dfd, path, &buf, (int)flag), err);
if (err == -1) {
throwUnixException(env, errno);
} else {
if (err == 0) {
copy_stat_attributes(env, &buf, attrs);
} else {
throwUnixException(env, errno);
}
}

Expand Down