RFR: 8343342: java/io/File/GetXSpace.java fails on Windows with CD-ROM drive

[Brian Burkhalter]

On Thu, 31 Oct 2024 07:15:16 GMT, Taizo Kurashige <duke at openjdk.org> wrote:

> To resolve java/io/File/GetXSpace.java failure, I fix libGetXSpace.c to use  Cygwin’s `df` to get the size for comparison if the test target drive is a CD-ROM drive.
> 
> As described in JDK-8343342, GetDiskSpaceInformationW can't get information about the size of the CD-ROM drive. 
> GetDiskFreeSpaceExW can also get information about the size of the CD-ROM drive. However, because GetDiskFreeSpaceExW is called by the File.get-X-Space methods, it seems more reasonable to compare the size got by other way than GetDiskFreeSpaceExW as a test. For this reason, I use Cygwin's `df`.
> In JDK-8298619, GetDiskSpaceInformationW was adopted instead of `df` because the size got by File.get-X-Space methods may not match the size got by `df` when per-user quotas are used. I don't think this problem applies to CD-ROM  drive, so I think we can use Cygwin's `df` for CD-ROM drive.
> 
> After fix, I ran a test on Windows Server 2019 where drive C is a normal local disk, drive D is an unmounted iso CD-ROM  drive, and drive F is an iso mounted CD-ROM drive and confirmed that it passes.
> 
> I think this fix may also resolves the similar failure reported at https://github.com/openjdk/jdk/pull/12397#issuecomment-1705164515.
> 
> Thanks

I verified that this proposed fix works on a Windows system with a CD-ROM drive attached, both when there is an actual CD in the drive and when the drive is empty.

What I don't like about this change is that the `df` command is executed from the native test code. I think it would be better if that were done in Java. Some restructuring would be needed.

Note that this smaller change versus the mainline test code fixes the case where an actual CD is in the drive:

--- a/test/jdk/java/io/File/libGetXSpace.c
+++ b/test/jdk/java/io/File/libGetXSpace.c
@@ -78,8 +78,9 @@ Java_GetXSpace_getSpace0
     }

     LPCWSTR path = (LPCWSTR)strchars;
+    UINT driveType = GetDriveTypeW(path);

-    if (pfnGetDiskSpaceInformation != NULL) {
+    if (pfnGetDiskSpaceInformation != NULL && driveType != DRIVE_CDROM) {
         // use GetDiskSpaceInformationW
         DISK_SPACE_INFORMATION diskSpaceInfo;
         BOOL hres = pfnGetDiskSpaceInformation(path, &diskSpaceInfo);

If there were also a simpler way to handle the empty drive case, that would be good.

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

PR Comment: https://git.openjdk.org/jdk/pull/21799#issuecomment-2505022527