Re: [libvirt] [PATCHv5 02/19] util: storagefile: Introduce universal function to canonicalize paths
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
Attachments:
- signature.asc (application/pgp-signature — 604 bytes)