Repository: netty/netty
Issue: Duplicate code in NativeLibraryLoader
Labels: cleanup

`NativeLibraryLoader` has some functions which are similar to `PlatformDependent`'s.

For example, 
- tmpdir()
- toDirectory()
- isWindow()

I wonder if these can be replaced with functions of `PlatformDependent`.
WDYT? 


Issue comments:
Fixed by https://github.com/netty/netty/pull/7058

Pull request review comments:
is this change necessary?
would it make sense to move the OS X platform detection to `PlatformDependent`?
yes as we use it in ´NativeLibraryLoader´
sure we can do but as there were no other places where this is used I did not bother. 
It looks like you removed all usages in this PR, or am I missing something?
Yip understood. My rational is if we need todo macOS specific stuff in the future `PlatformDependent` is the natural place to look, but we may miss it (and duplicate it) if the logic lives in this class. Since `PlatformDependent` is internal and duplication removal was the motivation for this PR it seemed like it warranted a discussion.
if that is true... which may be true that at the end I was able to remove all of them you are right... let me check
true, let me move it.
ok you are right... thanks :)

Patch that resolved the issue:
diff --git a/common/src/main/java/io/netty/util/internal/NativeLibraryLoader.java b/common/src/main/java/io/netty/util/internal/NativeLibraryLoader.java
index da8054a560c..729373506a6 100644
--- a/common/src/main/java/io/netty/util/internal/NativeLibraryLoader.java
+++ b/common/src/main/java/io/netty/util/internal/NativeLibraryLoader.java
@@ -31,7 +31,6 @@
 import java.security.PrivilegedAction;
 import java.util.Arrays;
 import java.util.EnumSet;
-import java.util.Locale;
 import java.util.Set;
 
 /**
@@ -43,13 +42,10 @@ public final class NativeLibraryLoader {
     private static final InternalLogger logger = InternalLoggerFactory.getInstance(NativeLibraryLoader.class);
 
     private static final String NATIVE_RESOURCE_HOME = "META-INF/native/";
-    private static final String OSNAME;
     private static final File WORKDIR;
     private static final boolean DELETE_NATIVE_LIB_AFTER_LOADING;
 
     static {
-        OSNAME = SystemPropertyUtil.get("os.name", "").toLowerCase(Locale.US).replaceAll("[^a-z0-9]+", "");
-
         String workdir = SystemPropertyUtil.get("io.netty.native.workdir");
         if (workdir != null) {
             File f = new File(workdir);
@@ -64,7 +60,7 @@ public final class NativeLibraryLoader {
             WORKDIR = f;
             logger.debug("-Dio.netty.native.workdir: " + WORKDIR);
         } else {
-            WORKDIR = tmpdir();
+            WORKDIR = PlatformDependent.tmpdir();
             logger.debug("-Dio.netty.native.workdir: " + WORKDIR + " (io.netty.tmpdir)");
         }
 
@@ -72,93 +68,6 @@ public final class NativeLibraryLoader {
                 "io.netty.native.deleteLibAfterLoading", true);
     }
 
-    private static File tmpdir() {
-        File f;
-        try {
-            f = toDirectory(SystemPropertyUtil.get("io.netty.tmpdir"));
-            if (f != null) {
-                logger.debug("-Dio.netty.tmpdir: " + f);
-                return f;
-            }
-
-            f = toDirectory(SystemPropertyUtil.get("java.io.tmpdir"));
-            if (f != null) {
-                logger.debug("-Dio.netty.tmpdir: " + f + " (java.io.tmpdir)");
-                return f;
-            }
-
-            // This shouldn't happen, but just in case ..
-            if (isWindows()) {
-                f = toDirectory(System.getenv("TEMP"));
-                if (f != null) {
-                    logger.debug("-Dio.netty.tmpdir: " + f + " (%TEMP%)");
-                    return f;
-                }
-
-                String userprofile = System.getenv("USERPROFILE");
-                if (userprofile != null) {
-                    f = toDirectory(userprofile + "\\AppData\\Local\\Temp");
-                    if (f != null) {
-                        logger.debug("-Dio.netty.tmpdir: " + f + " (%USERPROFILE%\\AppData\\Local\\Temp)");
-                        return f;
-                    }
-
-                    f = toDirectory(userprofile + "\\Local Settings\\Temp");
-                    if (f != null) {
-                        logger.debug("-Dio.netty.tmpdir: " + f + " (%USERPROFILE%\\Local Settings\\Temp)");
-                        return f;
-                    }
-                }
-            } else {
-                f = toDirectory(System.getenv("TMPDIR"));
-                if (f != null) {
-                    logger.debug("-Dio.netty.tmpdir: " + f + " ($TMPDIR)");
-                    return f;
-                }
-            }
-        } catch (Exception ignored) {
-            // Environment variable inaccessible
-        }
-
-        // Last resort.
-        if (isWindows()) {
-            f = new File("C:\\Windows\\Temp");
-        } else {
-            f = new File("/tmp");
-        }
-
-        logger.warn("Failed to get the temporary directory; falling back to: " + f);
-        return f;
-    }
-
-    @SuppressWarnings("ResultOfMethodCallIgnored")
-    private static File toDirectory(String path) {
-        if (path == null) {
-            return null;
-        }
-
-        File f = new File(path);
-        f.mkdirs();
-
-        if (!f.isDirectory()) {
-            return null;
-        }
-
-        try {
-            return f.getAbsoluteFile();
-        } catch (Exception ignored) {
-            return f;
-        }
-    }
-
-    private static boolean isWindows() {
-        return OSNAME.startsWith("windows");
-    }
-
-    private static boolean isOSX() {
-        return OSNAME.startsWith("macosx") || OSNAME.startsWith("osx");
-    }
-
     /**
      * Loads the first available library in the collection with the specified
      * {@link ClassLoader}.
@@ -208,7 +117,7 @@ public static void load(String originalName, ClassLoader loader) {
         String path = NATIVE_RESOURCE_HOME + libname;
 
         URL url = loader.getResource(path);
-        if (url == null && isOSX()) {
+        if (url == null && PlatformDependent.isOsx()) {
             if (path.endsWith(".jnilib")) {
                 url = loader.getResource(NATIVE_RESOURCE_HOME + "lib" + name + ".dynlib");
             } else {
diff --git a/common/src/main/java/io/netty/util/internal/PlatformDependent.java b/common/src/main/java/io/netty/util/internal/PlatformDependent.java
index e56ba5817cc..c97014edc5c 100644
--- a/common/src/main/java/io/netty/util/internal/PlatformDependent.java
+++ b/common/src/main/java/io/netty/util/internal/PlatformDependent.java
@@ -72,6 +72,8 @@ public final class PlatformDependent {
             "\\s*-XX:MaxDirectMemorySize\\s*=\\s*([0-9]+)\\s*([kKmMgG]?)\\s*$");
 
     private static final boolean IS_WINDOWS = isWindows0();
+    private static final boolean IS_OSX = isOsx0();
+
     private static final boolean MAYBE_SUPER_USER;
 
     private static final boolean CAN_ENABLE_TCP_NODELAY_BY_DEFAULT = !isAndroid();
@@ -207,6 +209,13 @@ public static boolean isWindows() {
         return IS_WINDOWS;
     }
 
+    /**
+     * Return {@code true} if the JVM is running on OSX / MacOS
+     */
+    public static boolean isOsx() {
+        return IS_OSX;
+    }
+
     /**
      * Return {@code true} if the current user may be a super-user. Be aware that this is just an hint and so it may
      * return false-positives.
@@ -923,6 +932,16 @@ private static boolean isWindows0() {
         return windows;
     }
 
+    private static boolean isOsx0() {
+        String osname = SystemPropertyUtil.get("os.name", "").toLowerCase(Locale.US);
+        boolean osx = osname.startsWith("macosx") || osname.startsWith("osx");
+
+        if (osx) {
+            logger.debug("Platform: MacOS");
+        }
+        return osx;
+    }
+
     private static boolean maybeSuperUser0() {
         String username = SystemPropertyUtil.get("user.name");
         if (isWindows()) {


Test patch: