Changeset 76589 in vbox for trunk

Timestamp:

Jan 1, 2019 8:56:07 AM (6 years ago)

Author:

vboxsync

Message:

IPRT: Attempted to address some the more obvious shortcomings of RTPathCalcRelative. Had to add a parameter that clearifies whether the from path is a file (VHD usage) or directory (rest).

Location:

trunk

Files:

• include/iprt/path.h (modified) (1 diff)
• src/VBox/Runtime/common/path/RTPathCalcRelative.cpp (modified) (2 diffs)
• src/VBox/Runtime/testcase/tstRTPath.cpp (modified) (2 diffs)
• src/VBox/Runtime/tools/RTDbgSymCache.cpp (modified) (1 diff)
• src/VBox/Storage/VHD.cpp (modified) (2 diffs)
• src/bldprogs/scmrw.cpp (modified) (1 diff)

trunk/include/iprt/path.h

@@ -1086 +1086 @@
  *                          terminator included.
  * @param   pszPathFrom     The path to start from creating the relative path.
+ * @param   fFromFile       Whether @a pszPathFrom is a file and we should work
+ *                          relative to it's parent directory (@c true), or if
+ *                          we should assume @a pszPathFrom is a directory and
+ *                          work relative to it.
  * @param   pszPathTo       The path to reach with the created relative path.
  */
-RTDECL(int) RTPathCalcRelative(char *pszPathDst, size_t cbPathDst,
-                               const char *pszPathFrom,
-                               const char *pszPathTo);
+RTDECL(int) RTPathCalcRelative(char *pszPathDst, size_t cbPathDst, const char *pszPathFrom, bool fFromFile, const char *pszPathTo);
 
 #ifdef IN_RING3

trunk/src/VBox/Runtime/common/path/RTPathCalcRelative.cpp

@@ -31 +31 @@
 #include "internal/iprt.h"
 #include <iprt/path.h>
+
 #include <iprt/assert.h>
+#if RTPATH_STYLE == RTPATH_STR_F_STYLE_DOS
+# include <iprt/ctype.h>
+#endif
 #include <iprt/err.h>
 #include <iprt/string.h>
@@ -39 +43 @@
 
 RTDECL(int) RTPathCalcRelative(char *pszPathDst, size_t cbPathDst,
-                               const char *pszPathFrom,
+                               const char *pszPathFrom, bool fFromFile,
                                const char *pszPathTo)
 {
-    int rc = VINF_SUCCESS;
-
     AssertPtrReturn(pszPathDst, VERR_INVALID_POINTER);
     AssertReturn(cbPathDst, VERR_INVALID_PARAMETER);
     AssertPtrReturn(pszPathFrom, VERR_INVALID_POINTER);
     AssertPtrReturn(pszPathTo, VERR_INVALID_POINTER);
-    AssertReturn(RTPathStartsWithRoot(pszPathFrom), VERR_INVALID_PARAMETER);
-    AssertReturn(RTPathStartsWithRoot(pszPathTo), VERR_INVALID_PARAMETER);
-    AssertReturn(RTStrCmp(pszPathFrom, pszPathTo), VERR_INVALID_PARAMETER);
 
     /*
      * Check for different root specifiers (drive letters), creating a relative path doesn't work here.
-     * @todo: How to handle case insensitive root specifiers correctly?
      */
     size_t offRootFrom = rtPathRootSpecLen(pszPathFrom);
+    AssertReturn(offRootFrom > 0, VERR_INVALID_PARAMETER);
+
     size_t offRootTo   = rtPathRootSpecLen(pszPathTo);
-
-    if (   offRootFrom != offRootTo
-        || RTStrNCmp(pszPathFrom, pszPathTo, offRootFrom))
+    AssertReturn(offRootTo > 0, VERR_INVALID_PARAMETER);
+
+
+    /** @todo correctly deal with extra root slashes! */
+    if (offRootFrom != offRootTo)
         return VERR_NOT_SUPPORTED;
 
-    /* Filter out the parent path which is equal to both paths. */
-    while (   *pszPathFrom == *pszPathTo
-           && *pszPathFrom != '\0'
-           && *pszPathTo != '\0')
-    {
-        pszPathFrom++;
-        pszPathTo++;
-    }
+#if RTPATH_STYLE != RTPATH_STR_F_STYLE_DOS
+    if (RTStrNCmp(pszPathFrom, pszPathTo, offRootFrom))
+        return VERR_NOT_SUPPORTED;
+#else
+    if (RTStrNICmp(pszPathFrom, pszPathTo, offRootFrom))
+        for (size_t off = 0; off < offRootFrom; off++)
+        {
+            char const chFrom = pszPathFrom[off];
+            char const chTo   = pszPathTo[off];
+            if (   chFrom != chTo
+                && RT_C_TO_LOWER(chFrom) != RT_C_TO_LOWER(chTo) /** @todo proper case insensitivity! */
+                && (!RTPATH_IS_SLASH(chFrom) || !RTPATH_IS_SLASH(chTo)) )
+                return VERR_NOT_SUPPORTED;
+        }
+#endif
+
+    pszPathFrom += offRootFrom;
+    pszPathTo   += offRootTo;
 
     /*
-     * Because path components can start with an equal string but differ afterwards we
-     * need to go back to the beginning of the current component.
+     * Skip out the part of the path which is equal to both.
      */
-    while (!RTPATH_IS_SEP(*pszPathFrom))
-        pszPathFrom--;
-
-    pszPathFrom++; /* Skip path separator. */
-
-    while (!RTPATH_IS_SEP(*pszPathTo))
-        pszPathTo--;
-
-    pszPathTo++; /* Skip path separator. */
+    const char *pszStartOfFromComp = pszPathFrom;
+    for (;;)
+    {
+        char const chFrom = *pszPathFrom;
+        char const chTo   = *pszPathTo;
+        if (!RTPATH_IS_SLASH(chFrom))
+        {
+            if (chFrom == chTo)
+            {
+                if (chFrom)
+                { /* likely */ }
+                else
+                {
+                    /* Special case: The two paths are equal. */
+                    if (fFromFile)
+                    {
+                        size_t cchComp = pszPathFrom - pszStartOfFromComp;
+                        if (cchComp < cbPathDst)
+                        {
+                            memcpy(pszPathDst, pszStartOfFromComp, cchComp);
+                            pszPathDst[cchComp] = '\0';
+                            return VINF_SUCCESS;
+                        }
+                    }
+                    else if (sizeof(".") <= cbPathDst)
+                    {
+                        memcpy(pszPathDst, ".", sizeof("."));
+                        return VINF_SUCCESS;
+                    }
+                    return VERR_BUFFER_OVERFLOW;
+                }
+            }
+#if RTPATH_STYLE == RTPATH_STR_F_STYLE_DOS
+            else if (RT_C_TO_LOWER(chFrom) == RT_C_TO_LOWER(chTo))
+            { /* if not likely, then simpler code structure wise. */ }
+#endif
+            else if (chFrom != '\0' || !RTPATH_IS_SLASH(chTo) || fFromFile)
+                break;
+            else
+            {
+                pszStartOfFromComp = pszPathFrom;
+                do
+                    pszPathTo++;
+                while (RTPATH_IS_SLASH(*pszPathTo));
+                break;
+            }
+            pszPathFrom++;
+            pszPathTo++;
+        }
+        else if (RTPATH_IS_SLASH(chTo))
+        {
+            /* Both have slashes.  Skip any additional ones before taking down
+               the start of the component for rewinding purposes. */
+            do
+                pszPathTo++;
+            while (RTPATH_IS_SLASH(*pszPathTo));
+            do
+                pszPathFrom++;
+            while (RTPATH_IS_SLASH(*pszPathFrom));
+            pszStartOfFromComp = pszPathFrom;
+        }
+        else
+            break;
+    }
+
+    /* Rewind to the start of the current component. */
+    pszPathTo  -= pszPathFrom - pszStartOfFromComp;
+    pszPathFrom = pszStartOfFromComp;
 
     /* Paths point to the first non equal component now. */
-    char aszPathTmp[RTPATH_MAX + 1];
-    unsigned offPathTmp = 0;
+
+    /*
+     * Constructure the relative path.
+     */
 
     /* Create the part to go up from pszPathFrom. */
+    unsigned offDst = 0;
+
+    if (!fFromFile && *pszPathFrom != '\0')
+    {
+        if (offDst + 3 < cbPathDst)
+        {
+            pszPathDst[offDst++] = '.';
+            pszPathDst[offDst++] = '.';
+            pszPathDst[offDst++] = RTPATH_SLASH;
+        }
+        else
+            return VERR_BUFFER_OVERFLOW;
+    }
+
     while (*pszPathFrom != '\0')
     {
-        while (   !RTPATH_IS_SEP(*pszPathFrom)
-               && *pszPathFrom != '\0')
+        char ch;
+        while (   (ch = *pszPathFrom) != '\0'
+               && !RTPATH_IS_SLASH(*pszPathFrom))
             pszPathFrom++;
-
-        if (RTPATH_IS_SEP(*pszPathFrom))
-        {
-            if (offPathTmp + 3 >= sizeof(aszPathTmp) - 1)
-                return VERR_FILENAME_TOO_LONG;
-            aszPathTmp[offPathTmp++] = '.';
-            aszPathTmp[offPathTmp++] = '.';
-            aszPathTmp[offPathTmp++] = RTPATH_SLASH;
+        while (   (ch = *pszPathFrom) != '\0'
+               && RTPATH_IS_SLASH(ch))
             pszPathFrom++;
-        }
-    }
-
-    aszPathTmp[offPathTmp] = '\0';
+        if (!ch)
+            break;
+
+        if (offDst + 3 < cbPathDst)
+        {
+            pszPathDst[offDst++] = '.';
+            pszPathDst[offDst++] = '.';
+            pszPathDst[offDst++] = RTPATH_SLASH;
+        }
+        else
+            return VERR_BUFFER_OVERFLOW;
+    }
 
     /* Now append the rest of pszPathTo to the final path. */
-    char *pszPathTmp = &aszPathTmp[offPathTmp];
-    size_t cbPathTmp = sizeof(aszPathTmp) - offPathTmp - 1;
-    rc = RTStrCatP(&pszPathTmp, &cbPathTmp, pszPathTo);
-    if (RT_SUCCESS(rc))
-    {
-        *pszPathTmp = '\0';
-
-        size_t cchPathTmp = strlen(aszPathTmp);
-        if (cchPathTmp >= cbPathDst)
-           return VERR_BUFFER_OVERFLOW;
-        memcpy(pszPathDst, aszPathTmp, cchPathTmp + 1);
-    }
-    else
-        rc = VERR_FILENAME_TOO_LONG;
-
-    return rc;
+    size_t cchTo = strlen(pszPathTo);
+    if (offDst + cchTo <= cbPathDst)
+    {
+        memcpy(&pszPathDst[offDst], pszPathTo, cchTo);
+        pszPathDst[offDst + cchTo] = '\0';
+        return VINF_SUCCESS;
+    }
+    return VERR_BUFFER_OVERFLOW;
 }
 

trunk/src/VBox/Runtime/testcase/tstRTPath.cpp

@@ -742 +742 @@
     {
         const char *pszFrom;
+        bool fFromFile;
         const char *pszTo;
         int rc;
@@ -747 +748 @@
     } s_aRelPath[] =
     {
-        { "/home/test.ext", "/home/test2.ext", VINF_SUCCESS, "test2.ext"},
-        { "/dir/test.ext", "/dir/dir2/test2.ext", VINF_SUCCESS, "dir2/test2.ext"},
-        { "/dir/dir2/test.ext", "/dir/test2.ext", VINF_SUCCESS, ".." RTPATH_SLASH_STR "test2.ext"},
-        { "/dir/dir2/test.ext", "/dir/dir3/test2.ext", VINF_SUCCESS, ".." RTPATH_SLASH_STR "dir3/test2.ext"},
+        { "/home/test.ext",     true,           "/home/test2.ext",        VINF_SUCCESS, "test2.ext" },
+        { "/dir/test.ext",      true,           "/dir/dir2/test2.ext",    VINF_SUCCESS, "dir2/test2.ext" },
+        { "/dir/dir2/test.ext", true,           "/dir/test2.ext",         VINF_SUCCESS, ".." RTPATH_SLASH_STR "test2.ext" },
+        { "/dir/dir2/test.ext", true,           "/dir/dir3/test2.ext",    VINF_SUCCESS, ".." RTPATH_SLASH_STR "dir3/test2.ext" },
+        { "/dir/dir2",          false,          "/dir/dir3/test2.ext",    VINF_SUCCESS, ".." RTPATH_SLASH_STR "dir3/test2.ext" },
+        { "/dir/dir2",          false,          "/dir/dir3//test2.ext",   VINF_SUCCESS, ".." RTPATH_SLASH_STR "dir3//test2.ext" },
+        { "/dir/dir2/",         false,          "/dir/dir3/test2.ext",    VINF_SUCCESS, ".." RTPATH_SLASH_STR "dir3/test2.ext" },
+        { "/dir/dir2////",      false,          "/dir//dir3/test2.ext",   VINF_SUCCESS, ".." RTPATH_SLASH_STR "dir3/test2.ext" },
+        { "/include/iprt",      false,          "/include/iprt/cdefs.h",  VINF_SUCCESS, "cdefs.h" },
+        { "/include/iprt/",     false,          "/include/iprt/cdefs.h",  VINF_SUCCESS, "cdefs.h" },
+        { "/include/iprt/tt.h", true,           "/include/iprt/cdefs.h",  VINF_SUCCESS, "cdefs.h" },
 #if defined (RT_OS_OS2) || defined (RT_OS_WINDOWS)
-        { "\\\\server\\share\\test.ext", "\\\\server\\share2\\test2.ext", VERR_NOT_SUPPORTED, ""},
-        { "c:\\dir\\test.ext", "f:\\dir\\test.ext", VERR_NOT_SUPPORTED, ""}
+        { "\\\\server\\share\\test.ext", true,  "\\\\server\\share2\\test2.ext", VERR_NOT_SUPPORTED, "" },
+        { "c:\\dir\\test.ext",  true,           "f:\\dir\\test.ext",      VERR_NOT_SUPPORTED, "" },
+        { "F:\\dir\\test.ext",  false,          "f:/dir//test.ext",      VINF_SUCCESS, "." } ,
+        { "F:\\dir\\test.ext",  true,           "f:/dir//test.ext",      VINF_SUCCESS, "test.ext" } ,
 #endif
     };
     for (unsigned i = 0; i < RT_ELEMENTS(s_aRelPath); i++)
     {
-        const char *pszFrom = s_aRelPath[i].pszFrom;
-        const char *pszTo   = s_aRelPath[i].pszTo;
-
-        rc = RTPathCalcRelative(szPath, sizeof(szPath), pszFrom, pszTo);
+        const char *pszFrom   = s_aRelPath[i].pszFrom;
+        bool        fFromFile = s_aRelPath[i].fFromFile;
+        const char *pszTo     = s_aRelPath[i].pszTo;
+
+        rc = RTPathCalcRelative(szPath, sizeof(szPath), pszFrom, fFromFile, pszTo);
         if (rc != s_aRelPath[i].rc)
-            RTTestIFailed("Unexpected return code\n"
+            RTTestIFailed("Unexpected return code for %s .. %s\n"
                           "     got: %Rrc\n"
                           "expected: %Rrc",
-                          rc, s_aRelPath[i].rc);
+                          pszFrom, pszTo, rc, s_aRelPath[i].rc);
         else if (   RT_SUCCESS(rc)
                  && strcmp(szPath, s_aRelPath[i].pszExpected))
             RTTestIFailed("Unexpected result\n"
-                          "    from: '%s'\n"
+                          "    from: '%s' (%s)\n"
                           "      to: '%s'\n"
                           "  output: '%s'\n"
                           "expected: '%s'",
-                          pszFrom, pszTo, szPath, s_aRelPath[i].pszExpected);
+                          pszFrom, fFromFile ? "file" : "dir", pszTo, szPath, s_aRelPath[i].pszExpected);
     }
 

trunk/src/VBox/Runtime/tools/RTDbgSymCache.cpp

@@ -240 +240 @@
     char szLinkTarget[RTPATH_MAX];
     //szMapPath[cch] = '\0';
-    rc = RTPathCalcRelative(szLinkTarget, sizeof(szLinkTarget), szMapPath, pszCacheFile);
+    rc = RTPathCalcRelative(szLinkTarget, sizeof(szLinkTarget), szMapPath, false /*fFromFile*/, pszCacheFile);
     //szMapPath[cch] = RTPATH_SLASH;
     if (RT_FAILURE(rc))

trunk/src/VBox/Storage/VHD.cpp

@@ -331 +331 @@
                 /* Convert to relative path. */
                 char szPath[RTPATH_MAX];
-                rc = RTPathCalcRelative(szPath, sizeof(szPath), pImage->pszFilename,
-                                        pszFilename);
+                rc = RTPathCalcRelative(szPath, sizeof(szPath), pImage->pszFilename, true /*fFromFile*/, pszFilename);
                 if (RT_SUCCESS(rc))
                 {
@@ -374 +373 @@
                 /* Convert to relative path. */
                 char szPath[RTPATH_MAX];
-                rc = RTPathCalcRelative(szPath, sizeof(szPath), pImage->pszFilename,
-                                        pszFilename);
+                rc = RTPathCalcRelative(szPath, sizeof(szPath), pImage->pszFilename, true /*fFromFile*/, pszFilename);
                 if (RT_SUCCESS(rc))
                     rc = vhdFilenameToUtf16(szPath, (uint16_t *)pvBuf, cbMaxLen, &cb, false);

trunk/src/bldprogs/scmrw.cpp

@@ -2896 +2896 @@
         else
             rc = RTPathCalcRelative(&szNormalized[cchNormalized], sizeof(szNormalized) - cchNormalized,
-                                    pSettings->pszGuardRelativeToDir, pState->pszFilename);
+                                    pSettings->pszGuardRelativeToDir, false /*fFromFile*/, pState->pszFilename);
         if (RT_FAILURE(rc))
             return ScmError(pState, rc, "Error calculating guard prefix (RTPathCalcRelative): %Rrc\n", rc);