On Wed, 29 Mar 2023 16:10:34 GMT, Roger Riggs <rri...@openjdk.org> wrote:
>> Brian Burkhalter has updated the pull request with a new target base due to
>> a merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains five additional
>> commits since the last revision:
>>
>> - 8298619: Change getSpace0() to return a boolean; print warning if total
>> size is estimated instead of exact
>> - Merge
>> - 8298619: Load GetDiskSpaceInformationW dynamically
>> - 8298619: Replace df and diskFree with native calls
>> - 8298619: java/io/File/GetXSpace.java is failing
>
> test/jdk/java/io/File/GetXSpace.java line 278:
>
>> 276: fail(s.name() + " free space (quota)", fs, ">",
>> s.size());
>> 277: } else {
>> 278: pass();
>
> Can the inverted check and if/else clauses be swapped. It would read more
> naturally as:
>
> if (windows()) {
> ...windows code...
> } else {
> ... non-windows code...
> }
So changed in 9ae317e7b485d4ac56be25731165aafef694e9b5.
> test/jdk/java/io/File/libGetXSpace.c line 66:
>
>> 64: if (chars == NULL) {
>> 65: JNU_ThrowByNameWithLastError(env, "java/lang/RuntimeException",
>> 66: "GetStringChars");
>
> Add a `return JNI_FALSE`; otherwise it falls through.
Fixed in 9ae317e7b485d4ac56be25731165aafef694e9b5.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/12397#discussion_r1156137512
PR Review Comment: https://git.openjdk.org/jdk/pull/12397#discussion_r1156137241
