8192927: os::dir_is_empty is incorrect on Windows
authorccheung
Thu, 04 Jan 2018 22:47:38 -0800
changeset 48611 4d7a4fad8190
parent 48489 a5548cf24286
child 48612 9e524244b67d
8192927: os::dir_is_empty is incorrect on Windows Summary: Check file names in a directory. It is empty if only the "." and ".." files exist. Use unicode version of windows APIs to handle long path. Reviewed-by: iklam, sspitsyn
src/hotspot/os/windows/os_windows.cpp
test/hotspot/jtreg/runtime/appcds/DirClasspathTest.java
--- a/src/hotspot/os/windows/os_windows.cpp	Thu Jan 04 22:54:40 2018 -0500
+++ b/src/hotspot/os/windows/os_windows.cpp	Thu Jan 04 22:47:38 2018 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2018, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -4394,13 +4394,49 @@
 
 // Is a (classpath) directory empty?
 bool os::dir_is_empty(const char* path) {
-  WIN32_FIND_DATA fd;
-  HANDLE f = FindFirstFile(path, &fd);
-  if (f == INVALID_HANDLE_VALUE) {
-    return true;
-  }
-  FindClose(f);
-  return false;
+  char* search_path = (char*)os::malloc(strlen(path) + 3, mtInternal);
+  if (search_path == NULL) {
+    errno = ENOMEM;
+    return false;
+  }
+  strcpy(search_path, path);
+  // Append "*", or possibly "\\*", to path
+  if (path[1] == ':' &&
+    (path[2] == '\0' ||
+    (path[2] == '\\' && path[3] == '\0'))) {
+    // No '\\' needed for cases like "Z:" or "Z:\"
+    strcat(search_path, "*");
+  }
+  else {
+    strcat(search_path, "\\*");
+  }
+  errno_t err = ERROR_SUCCESS;
+  wchar_t* wpath = create_unc_path(search_path, err);
+  if (err != ERROR_SUCCESS) {
+    if (wpath != NULL) {
+      destroy_unc_path(wpath);
+    }
+    os::free(search_path);
+    errno = err;
+    return false;
+  }
+  WIN32_FIND_DATAW fd;
+  HANDLE f = ::FindFirstFileW(wpath, &fd);
+  destroy_unc_path(wpath);
+  bool is_empty = true;
+  if (f != INVALID_HANDLE_VALUE) {
+    while (is_empty && ::FindNextFileW(f, &fd)) {
+      // An empty directory contains only the current directory file
+      // and the previous directory file.
+      if ((wcscmp(fd.cFileName, L".") != 0) &&
+          (wcscmp(fd.cFileName, L"..") != 0)) {
+        is_empty = false;
+      }
+    }
+    FindClose(f);
+  }
+  os::free(search_path);
+  return is_empty;
 }
 
 // create binary file, rewriting existing file if required
--- a/test/hotspot/jtreg/runtime/appcds/DirClasspathTest.java	Thu Jan 04 22:54:40 2018 -0500
+++ b/test/hotspot/jtreg/runtime/appcds/DirClasspathTest.java	Thu Jan 04 22:47:38 2018 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2018, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -33,8 +33,13 @@
 import jdk.test.lib.Platform;
 import jdk.test.lib.process.OutputAnalyzer;
 import java.io.File;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Arrays;
 
 public class DirClasspathTest {
+    private static final int MAX_PATH = 260;
+
     public static void main(String[] args) throws Exception {
         File dir = new File(System.getProperty("user.dir"));
         File emptydir = new File(dir, "emptydir");
@@ -42,15 +47,36 @@
 
         // Empty dir in -cp: should be OK
         OutputAnalyzer output;
-        if (!Platform.isWindows()) {
-            // This block fails on Windows because of JDK-8192927
-            output = TestCommon.dump(emptydir.getPath(), TestCommon.list("DoesntMatter"), "-Xlog:class+path=info");
-            TestCommon.checkDump(output);
+        String classList[] = {"java/lang/Object"};
+        output = TestCommon.dump(emptydir.getPath(), classList, "-Xlog:class+path=info");
+        TestCommon.checkDump(output);
+
+        // Long path to empty dir in -cp: should be OK
+        Path classDir = Paths.get(System.getProperty("test.classes"));
+        Path destDir = classDir;
+        int subDirLen = MAX_PATH - classDir.toString().length() - 2;
+        if (subDirLen > 0) {
+            char[] chars = new char[subDirLen];
+            Arrays.fill(chars, 'x');
+            String subPath = new String(chars);
+            destDir = Paths.get(System.getProperty("test.classes"), subPath);
         }
+        File longDir = destDir.toFile();
+        longDir.mkdir();
+        File subDir = new File(longDir, "subdir");
+        subDir.mkdir();
+        output = TestCommon.dump(subDir.getPath(), classList, "-Xlog:class+path=info");
+        TestCommon.checkDump(output);
 
         // Non-empty dir in -cp: should fail
         // <dir> is not empty because it has at least one subdirectory, i.e., <emptydir>
-        output = TestCommon.dump(dir.getPath(), TestCommon.list("DoesntMatter"), "-Xlog:class+path=info");
+        output = TestCommon.dump(dir.getPath(), classList, "-Xlog:class+path=info");
+        output.shouldNotHaveExitValue(0);
+        output.shouldContain("CDS allows only empty directories in archived classpaths");
+
+        // Long path to non-empty dir in -cp: should fail
+        // <dir> is not empty because it has at least one subdirectory, i.e., <emptydir>
+        output = TestCommon.dump(longDir.getPath(), classList, "-Xlog:class+path=info");
         output.shouldNotHaveExitValue(0);
         output.shouldContain("CDS allows only empty directories in archived classpaths");
     }