On 06/20/14 16:35, Eric Blake wrote:
On 06/19/2014 07:59 AM, Peter Krempa wrote:
Introduce a common function that will take a callback to resolve links that will be used to canonicalize paths on various storage systems and add extensive tests. --- src/libvirt_private.syms | 1 + src/util/virstoragefile.c | 195 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/virstoragefile.h | 7 ++ tests/virstoragetest.c | 108 +++++++++++++++++++++++++ 4 files changed, 311 insertions(+)
ACK if you can make those changes (you may want to post the interdiff)
The required changes to pass your suggested changes are: diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index be33398..613ba3c 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2040,6 +2040,7 @@ virStorageFileCanonicalizePath(const char *path, char *linkpath = NULL; char *currentpath = NULL; size_t i = 0; + size_t j = 0; int rc; char *ret = NULL; @@ -2056,10 +2057,21 @@ virStorageFileCanonicalizePath(const char *path, if (!(components = virStringSplitCount(path, "/", 0, &ncomponents))) goto cleanup; + j = 0; + while (j < ncomponents) { + /* skip slashes */ + if (STREQ(components[j], "")) { + VIR_FREE(components[j]); + VIR_DELETE_ELEMENT(components, j, ncomponents); + continue; + } + j++; + } + while (i < ncomponents) { - /* skip slashes and '.'s */ - if (STREQ(components[i], "") || - STREQ(components[i], ".")) { + /* skip '.'s unless it's the last one remaining */ + if (STREQ(components[i], ".") && + (beginSlash || ncomponents > 1)) { VIR_FREE(components[i]); VIR_DELETE_ELEMENT(components, i, ncomponents); continue; @@ -2131,6 +2143,17 @@ virStorageFileCanonicalizePath(const char *path, &ncomponents) < 0) goto cleanup; + j = 0; + while (j < ncomponents) { + /* skip slashes */ + if (STREQ(components[j], "")) { + VIR_FREE(components[j]); + VIR_DELETE_ELEMENT(components, j, ncomponents); + continue; + } + j++; + } + VIR_FREE(linkpath); VIR_FREE(currentpath); diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 0bc4a42..f86d25c 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1154,14 +1154,14 @@ mymain(void) TEST_PATH_CANONICALIZE(5, "///////", "/"); TEST_PATH_CANONICALIZE(6, "//", "//"); TEST_PATH_CANONICALIZE(7, "", ""); - TEST_PATH_CANONICALIZE(8, ".", ""); + TEST_PATH_CANONICALIZE(8, ".", "."); TEST_PATH_CANONICALIZE(9, "../", ".."); TEST_PATH_CANONICALIZE(10, "../../", "../.."); TEST_PATH_CANONICALIZE(11, "../../blah", "../../blah"); TEST_PATH_CANONICALIZE(12, "/./././blah", "/blah"); TEST_PATH_CANONICALIZE(13, ".././../././../blah", "../../../blah"); TEST_PATH_CANONICALIZE(14, "/././", "/"); - TEST_PATH_CANONICALIZE(15, "./././", ""); + TEST_PATH_CANONICALIZE(15, "./././", "."); TEST_PATH_CANONICALIZE(16, "blah/../foo", "foo"); TEST_PATH_CANONICALIZE(17, "foo/bar/../blah", "foo/blah"); TEST_PATH_CANONICALIZE(18, "foo/bar/.././blah", "foo/blah"); @@ -1179,6 +1179,7 @@ mymain(void) TEST_PATH_CANONICALIZE(28, "/path/blah/yippee", "/other/path/huzah/yippee"); TEST_PATH_CANONICALIZE(29, "/cycle", NULL); TEST_PATH_CANONICALIZE(30, "/cycle2/link", NULL); + TEST_PATH_CANONICALIZE(31, "///", "/"); cleanup: /* Final cleanup */ Peter