util: Fix abs_file_name() bugs on Windows.

abs_file_name() believed that a file name that begins with / or contains :
is absolute and that any other file name is relative.  On Windows, this is
wrong in at least the following ways:

   * / and \ are interchangeable on Windows.

   * A name that begins with \\ or // is also absolute.

   * A name that begins with X: but not X:\ is not absolute.

   * A name with : in some position other than the second position is
     not absolute (although it might not be valid either?).

Furthermore, Windows has more than one current working directory (one per
volume letter), so trying to make a file name absolute by just prefixing
the current working directory for the current volume results in silliness.

This patch attempts to fix the problem.

This makes OVS link against shlwapi, which is needed to use
PathIsRelative().

Found by inspection.

Acked-by: Alin Gabriel Serdean <aserdean@ovn.org>
Signed-off-by: Ben Pfaff <blp@ovn.org>

4 files changed, 46 insertions, 23 deletions

diff --git a/Documentation/intro/install/windows.rst b/Documentation/intro/install/windows.rst
index 6d8c746c5..f696d2c9b 100644
--- a/Documentation/intro/install/windows.rst
+++ b/Documentation/intro/install/windows.rst
@@ -156,7 +156,7 @@ component installation directories, etc. For example:
 ::
 
    $ ./configure CC=./build-aux/cccl LD="$(which link)" \
-       LIBS="-lws2_32 -liphlpapi -lwbemuuid -lole32 -loleaut32" \
+       LIBS="-lws2_32 -lShlwapi -liphlpapi -lwbemuuid -lole32 -loleaut32" \
        --prefix="C:/openvswitch/usr" \
        --localstatedir="C:/openvswitch/var" \
        --sysconfdir="C:/openvswitch/etc" \
@@ -172,7 +172,7 @@ To configure with SSL support, add the requisite additional options:
 ::
 
    $ ./configure CC=./build-aux/cccl LD="`which link`"  \
-       LIBS="-lws2_32 -liphlpapi -lwbemuuid -lole32 -loleaut32" \
+       LIBS="-lws2_32 -lShlwapi -liphlpapi -lwbemuuid -lole32 -loleaut32" \
        --prefix="C:/openvswitch/usr" \
        --localstatedir="C:/openvswitch/var"
        --sysconfdir="C:/openvswitch/etc" \
@@ -184,7 +184,7 @@ Finally, to the kernel module also:
 ::
 
    $ ./configure CC=./build-aux/cccl LD="`which link`" \
-       LIBS="-lws2_32 -liphlpapi -lwbemuuid -lole32 -loleaut32" \
+       LIBS="-lws2_32 -lShlwapi -liphlpapi -lwbemuuid -lole32 -loleaut32" \
        --prefix="C:/openvswitch/usr" \
        --localstatedir="C:/openvswitch/var" \
        --sysconfdir="C:/openvswitch/etc" \
diff --git a/appveyor.yml b/appveyor.yml
index 49fcae4b5..2e5c37a37 100644
--- a/appveyor.yml
+++ b/appveyor.yml
@@ -41,6 +41,6 @@ build_script:
 - C:\MinGW\msys\1.0\bin\bash -lc "cp /c/pthreads-win32/Pre-built.2/dll/x86/*.dll /c/openvswitch/."
 - C:\MinGW\msys\1.0\bin\bash -lc "mv /bin/link.exe /bin/link_copy.exe"
 - C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && ./boot.sh"
-- C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && ./configure CC=build-aux/cccl LD=\"`which link`\" LIBS=\"-lws2_32 -liphlpapi -lwbemuuid -lole32 -loleaut32\" --with-pthread=C:/pthreads-win32/Pre-built.2 --with-openssl=C:/OpenSSL-Win32 --with-vstudiotarget=\"Debug\"
+- C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && ./configure CC=build-aux/cccl LD=\"`which link`\" LIBS=\"-lws2_32 -lShlwapi -liphlpapi -lwbemuuid -lole32 -loleaut32\" --with-pthread=C:/pthreads-win32/Pre-built.2 --with-openssl=C:/OpenSSL-Win32 --with-vstudiotarget=\"Debug\"
 - C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && make"
 - C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && make datapath_windows_analyze"
diff --git a/lib/util.c b/lib/util.c
index 7152b5539..082bc7bb0 100644
--- a/lib/util.c
+++ b/lib/util.c
@@ -38,6 +38,9 @@
 #ifdef HAVE_PTHREAD_SET_NAME_NP
 #include <pthread_np.h>
 #endif
+#ifdef _WIN32
+#include <shlwapi.h>
+#endif
 
 VLOG_DEFINE_THIS_MODULE(util);
 
@@ -1049,37 +1052,56 @@ base_name(const char *file_name)
 }
 #endif /* _WIN32 */
 
-/* If 'file_name' starts with '/', returns a copy of 'file_name'.  Otherwise,
+bool
+is_file_name_absolute(const char *fn)
+{
+#ifdef _WIN32
+    /* Use platform specific API */
+    return !PathIsRelative(fn);
+#else
+    /* An absolute path begins with /. */
+    return fn[0] == '/';
+#endif
+}
+
+/* If 'file_name' is absolute, returns a copy of 'file_name'.  Otherwise,
  * returns an absolute path to 'file_name' considering it relative to 'dir',
  * which itself must be absolute.  'dir' may be null or the empty string, in
  * which case the current working directory is used.
  *
- * Additionally on Windows, if 'file_name' has a ':', returns a copy of
- * 'file_name'
- *
  * Returns a null pointer if 'dir' is null and getcwd() fails. */
 char *
 abs_file_name(const char *dir, const char *file_name)
 {
-    if (file_name[0] == '/') {
-        return xstrdup(file_name);
-#ifdef _WIN32
-    } else if (strchr(file_name, ':')) {
+    /* If it's already absolute, return a copy. */
+    if (is_file_name_absolute(file_name)) {
         return xstrdup(file_name);
-#endif
-    } else if (dir && dir[0]) {
+    }
+
+    /* If a base dir was supplied, use it.  We assume, without checking, that
+     * the base dir is absolute.*/
+    if (dir && dir[0]) {
         char *separator = dir[strlen(dir) - 1] == '/' ? "" : "/";
         return xasprintf("%s%s%s", dir, separator, file_name);
-    } else {
-        char *cwd = get_cwd();
-        if (cwd) {
-            char *abs_name = xasprintf("%s/%s", cwd, file_name);
-            free(cwd);
-            return abs_name;
-        } else {
-            return NULL;
-        }
     }
+
+#if _WIN32
+    /* It's a little complicated to make an absolute path on Windows because a
+     * relative path might still specify a drive letter.  The OS has a function
+     * to do the job for us, so use it. */
+    char abs_path[MAX_PATH];
+    DWORD n = GetFullPathName(file_name, sizeof abs_path, abs_path, NULL);
+    return n > 0 && n <= sizeof abs_path ? xmemdup0(abs_path, n) : NULL;
+#else
+    /* Outside Windows, do the job ourselves. */
+    char *cwd = get_cwd();
+    if (!cwd) {
+        return NULL;
+    }
+    char *abs_name = xasprintf("%s/%s", cwd, file_name);
+    free(cwd);
+    return abs_name;
+#endif
 }
 
 /* Like readlink(), but returns the link name as a null-terminated string in
diff --git a/lib/util.h b/lib/util.h
index 0061bb986..fd414489c 100644
--- a/lib/util.h
+++ b/lib/util.h
@@ -231,6 +231,7 @@ char *dir_name(const char *file_name);
 char *base_name(const char *file_name);
 #endif
 char *abs_file_name(const char *dir, const char *file_name);
+bool is_file_name_absolute(const char *);
 
 char *follow_symlinks(const char *filename);