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(+)
+static int +virStorageFileCanonicalizeInjectSymlink(const char *path, + size_t at, + char ***components, + size_t *ncomponents) +{ + char **tmp = NULL; + char **next; + size_t ntmp = 0; + int ret = -1; + + if (!(tmp = virStringSplitCount(path, "/", 0, &ntmp))) + goto cleanup; + + /* prepend */ + for (next = tmp; *next; next++) { + if (VIR_INSERT_ELEMENT(*components, at, *ncomponents, *next) < 0) + goto cleanup;
Hmm. Laine's VIR_INSERT_ macros have so far only been used to insert one element at a time, but the underlying functions that the macros call are capable of inserting an array of elements in one go, which is more efficient. I wonder if it's worth adding a macro to expose insertion of an array, which we could then use here. But I won't insist - your series is already long enough that it could be done another day.
+ + TEST_PATH_CANONICALIZE(1, "/", "/"); + TEST_PATH_CANONICALIZE(2, "/path", "/path"); + TEST_PATH_CANONICALIZE(3, "/path/to/blah", "/path/to/blah"); + TEST_PATH_CANONICALIZE(4, "/path/", "/path"); + TEST_PATH_CANONICALIZE(5, "///////", "/"); + TEST_PATH_CANONICALIZE(6, "//", "//");
Please also test: "///" canonicalizes to "/".
+ TEST_PATH_CANONICALIZE(7, "", ""); + TEST_PATH_CANONICALIZE(8, ".", "");
I'm a bit fuzzy on whether this is a good idea; the empty string is required to fail with ENOENT, while a lone "." is worth preserving as-is. I think test 7 is okay, but if you could tweak the code to make test 8 return ".", I'd be happier (that is, special case the deletion of "." to not do that if it is the only remaining element in the array).
+ 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, "./././", "");
Another case where I'd be happier if this returns "." (so again, if you special case that "." is deleted unless it is the last element of the simplified array)
+ TEST_PATH_CANONICALIZE(16, "blah/../foo", "foo"); + TEST_PATH_CANONICALIZE(17, "foo/bar/../blah", "foo/blah");
ACK if you can make those changes (you may want to post the interdiff) -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org