On 05/20/2010 03:28 PM, Eric Blake wrote: > On 05/20/2010 10:04 AM, Cole Robinson wrote: >> Spurious / in a pool target path makes life difficult for apps using the >> GetVolByPath, and doing other path based comparisons with pools. This >> has caused a few issues for virt-manager users: >> >> https://bugzilla.redhat.com/show_bug.cgi?id=494005 >> https://bugzilla.redhat.com/show_bug.cgi?id=593565 >> >> Add a new util API which removes spurious /, virFileSanitizePath. Sanitize >> target paths when parsing pool XML, and for paths passed to GetVolByPath. > > In general, I like the idea, whether or not we should be moving the > sanitization into a gnulib function. But I don't think it's ready for > an ack quite yet. > >> index e937d39..8f86ed6 100644 >> --- a/src/util/util.c >> +++ b/src/util/util.c >> @@ -1921,6 +1921,41 @@ int virFileAbsPath(const char *path, char **abspath) >> return 0; >> } >> >> +/* Remove spurious / characters from a path. The result must be freed */ >> +char * >> +virFileSanitizePath(const char *path) >> +{ >> + const char *cur = path; >> + char *cleanpath; >> + int idx = 0; >> + >> + cleanpath = strdup(path); >> + if (!cleanpath) { >> + virReportOOMError(); >> + return NULL; >> + } >> + >> + /* Sanitize path in place */ >> + while (*cur != '\0') { >> + if (*cur == '/') { >> + /* Skip all extra / */ >> + while (*++cur == '/') >> + continue; > > POSIX says that you must be careful of "//file" - you cannot blindly > simplify to "/file" (case in point - cygwin). But POSIX also says that > "///file" sanitizes to "/file". > >> + >> + /* Don't add a trailing / */ >> + if (*cur == '\0') >> + break; >> + >> + cleanpath[idx++] = '/'; >> + } >> + >> + cleanpath[idx++] = *cur++; >> + } >> + cleanpath[idx] = '\0'; >> + >> + return cleanpath; > > Sanitizing "a/b/../c" to "a/c" might not always be the right thing (what > if a is a symlink?), but it would also be safe to sanitize "a/./b" to "a/b". > >> +} >> + >> /* Like strtol, but produce an "int" result, and check more carefully. >> Return 0 upon success; return -1 to indicate failure. >> When END_PTR is NULL, the byte after the final valid digit must be NUL. >> diff --git a/src/util/util.h b/src/util/util.h >> index 6bf6bcc..abc2688 100644 >> --- a/src/util/util.h >> +++ b/src/util/util.h >> @@ -118,6 +118,8 @@ char *virFindFileInPath(const char *file); >> >> int virFileExists(const char *path); >> >> +char *virFileSanitizePath(const char *path); > > Mark this with ATTRIBUTE_NONNULL(1). > Thanks for the review, updated patch sent. - Cole -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list