RFR: 8338884: java/nio/file/attribute/BasicFileAttributeView/CreationTime.java#tmp fails on alinux3 [v15]

[Severin Gehwolf]

On Mon, 9 Sep 2024 02:35:38 GMT, SendaoYan <syan at openjdk.org> wrote:

>> Hi all,
>> On alinux3(alibaba cloud linux version 3) system, the `/tmp` disk partition is mounted as tmpfs filesystem type, this filesystem type doesn't support create time(birth time).
>> 
>> Before this PR, this test [check](https://github.com/openjdk/jdk/blob/master/test/jdk/java/nio/file/attribute/BasicFileAttributeView/CreationTime.java#L110) if there is `statx` system call present or not to determise the test environment support birth time or not. I think it's not enough when the tested filesystem type is `tmpfs`. When the tested filesystem type is `tmpfs`, then the tested file doesn't support birth time.
>> 
>> On RHEL 8 tmpfs doesn't seem to support birth time, but on F39 tmpfs does seem to support birth time. Looks like this might be related to the kernel version. It's difficult to enumerate all the combination of file system type and linux kernel version to determine the testd file support birth time or not. So in this PR, I get the result from `statx` linux syscall, to determine the testd file support birth time or not.
>> 
>> Test fix only, the change has been verified, risk is low.
>> 
>> Additional test:
>> 
>> - [x] Alinux3 glibc:2.32
>> 1.  /tmp/file supportsCreationTimeRead == false
>> 2. ./file supportsCreationTimeRead == true
>> - [x] CentOS7 docker container glibc:2.17
>> 1.  /tmp/file supportsCreationTimeRead == false
>> 2. ./file supportsCreationTimeRead == false
>
> SendaoYan has updated the pull request incrementally with one additional commit since the last revision:
> 
>   After JEP 347, we can use bool type only "#include <stdbool.h>"

Seems mostly fine now.

test/jdk/java/nio/file/attribute/BasicFileAttributeView/CreationTimeHelper.java line 45:

> 43: 
> 44:     // Helper so as to determine 'statx' support on the runtime system
> 45:     // static boolean linuxIsCreationTimeSupported(String file);

This comment is a bit misleading.

Suggestion:

    // Helper so as to determine birth time support on Linux.
    // Support is determined in a two-step process:
    // 1. Determine if `statx` system call is available. If available proceed, otherwise
    //    return false.
    // 2. Perform an actual `statx` call on the given file and check for birth time support
    //    in the mask returned from the call. This is needed, since some file systems, like
    //    nfs, don't support birth time even though the `statx` system call is available.

test/jdk/java/nio/file/attribute/BasicFileAttributeView/CreationTimeHelper.java line 48:

> 46:     static boolean linuxIsCreationTimeSupported(String file) throws Throwable {
> 47:         if (!abi.defaultLookup().find("statx").isPresent())
> 48:             return false;

I personally prefer using `if (...) { /* do something */ }` (add curly brackets).

test/jdk/java/nio/file/attribute/BasicFileAttributeView/libCreationTimeHelper.c line 91:

> 89:     struct my_statx stx;
> 90:     int ret, atflag = AT_SYMLINK_NOFOLLOW;
> 91:     memset(&stx, 0xbf, sizeof(stx));

Why `0xbf`?

How about `struct my_statx stx = {0}`?

-------------

PR Review: https://git.openjdk.org/jdk/pull/20687#pullrequestreview-2289152795
PR Review Comment: https://git.openjdk.org/jdk/pull/20687#discussion_r1749783056
PR Review Comment: https://git.openjdk.org/jdk/pull/20687#discussion_r1749785765
PR Review Comment: https://git.openjdk.org/jdk/pull/20687#discussion_r1749813293