[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libguestfs] [PATCH 1/3] New API: case-sensitive-path to return case sensitive path on NTFS 3g fs



On 26/10/09 09:20, Richard W.M. Jones wrote:
+char *
+do_case_sensitive_path (const char *path)
+{
+  char ret[PATH_MAX+1] = "/";

Is PATH_MAX on Windows <= POSIX PATH_MAX? Does ntfs_3g honour this? Seems like a grey area. It would be safer to realloc this buffer as necessary. You also wouldn't need the strdup() at the end.


+  size_t next = 1;
+
+  /* MUST chdir ("/") before leaving this function. */
+  if (chdir (sysroot) == -1) {
+    reply_with_perror ("%s", sysroot);
+    return NULL;
+  }

I'm not convinced chdir is necessary in this function if you use openat() throughout.

+  /* First character is a '/'.  Take each subsequent path element
+   * and follow it.
+   */

A check that *path == '/' wouldn't go amiss here. I'd stick an assert in, but then DV might shoot me ;)

+  path++;
+  while (*path) {
+    size_t i = strcspn (path, "/");
+    if (i == 0) {               /* "//" in path */
+      path++;
+      continue;
+    }
+    if ((i == 1&&  path[0] == '.') ||
+        (i == 2&&  path[0] == '.'&&  path[1] == '.')) {
+      reply_with_error ("case_sensitive_path: path contained . or .. elements");

This string isn't internationalised. I won't mention all of these.

+      goto error;
+    }
+    if (i>  NAME_MAX) {
+      reply_with_error ("case_sensitive_path: path element too long");
+      goto error;
+    }
+
+    char name[NAME_MAX+1];
+    memcpy (name, path, i);
+    name[i] = '\0';
+
+    /* Skip to next element in path (for the next loop iteration). */
+    path += i;
+    if (*path == '/') path++;
+
+    /* Read the current directory looking (case insensitively) for
+     * this element of the path.
+     */
+    DIR *dir = opendir (".");
+    if (dir == NULL) {
+      reply_with_perror ("opendir");
+      goto error;
+    }
+
+    struct dirent *d = NULL;
+
+    while ((d = readdir (dir)) != NULL) {
+      if (strcasecmp (d->d_name, name) == 0)
+        break;
+    }
+
+    if (closedir (dir) == -1) {
+      reply_with_perror ("closedir");
+      goto error;
+    }
+
+    if (d == NULL) {
+      reply_with_error ("%s: no file or directory found with this name", name);

Non-internationalised string. This message is definitely for the user.

+      goto error;
+    }
+
+    /* Add the real name of this path element to the return value. */
+    if (next>  1)
+      ret[next++] = '/';
+
+    i = strlen (d->d_name);
+    if (next + i>= PATH_MAX) {
+      reply_with_error ("final path too long");
+      goto error;
+    }
+
+    strcpy (&ret[next], d->d_name);
+    next += i;
+
+    /* Is it a directory?  Try going into it. */
+    if (chdir (d->d_name) == 0)
+      continue;

openat() and fstat() would be much nicer here.

+
+    /* This is OK provided we've reached the end of the path. */
+    if (errno == ENOTDIR) {
+      if (*path) {
+        reply_with_error ("non-directory element in path");
+        goto error;
+      }
+      break;
+    }
+  }
+
+  ignore_value (chdir ("/"));
+
+  ret[next] = '\0';
+  char *retp = strdup (ret);
+  if (retp == NULL) {
+    reply_with_perror ("strdup");
+    return NULL;
+  }
+  return retp;                  /* caller frees */
+
+ error:
+  ignore_value (chdir ("/"));
+  return NULL;
+}
diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR
index 0f11735..5381652 100644
--- a/src/MAX_PROC_NR
+++ b/src/MAX_PROC_NR
@@ -1 +1 @@
-196
+197
diff --git a/src/generator.ml b/src/generator.ml
index cea5178..668002e 100755
--- a/src/generator.ml
+++ b/src/generator.ml
@@ -3645,6 +3645,45 @@ The result list is not sorted.

  =back");

+  ("case_sensitive_path", (RString "rpath", [Pathname "path"]), 197, [],
+   [InitISOFS, Always, TestOutput (
+      [["case_sensitive_path"; "/DIRECTORY"]], "/directory");
+    InitISOFS, Always, TestOutput (
+      [["case_sensitive_path"; "/Known-1"]], "/known-1");
+    InitBasicFS, Always, TestOutput (
+      [["mkdir"; "/a"];
+       ["mkdir"; "/a/bbb"];
+       ["touch"; "/a/bbb/c"];
+       ["case_sensitive_path"; "/A/bbB/C"]], "/a/bbb/c")],

These don't exercise the following cases you've coded for:

/
/foo//bar
/foo/../bar
/foo///bar/

+   "return true path on case-insensitive filesystem",
+   "\
+This command handles a peculiarity of the Linux ntfs-3g
+filesystem driver (and probably others), which is that although
+the underlying filesystem is case-insensitive, the driver
+exports the filesystem to Linux as case-sensitive.
+
+One consequence of this is that special directories such
+as C<c:\\windows>  may appear as C</WINDOWS>  or C</windows>
+(or other things) depending on the precise details of how
+they were created.  In Windows itself this would not be
+a problem.
+
+Bug or feature?  You decide:
+L<http://www.tuxera.com/community/ntfs-3g-faq/#posixfilenames1>

It's definitely a bug ;)

+
+This function resolves the true case of each element in the
+path and returns the case-sensitive path.
+
+Thus C<guestfs_case_sensitive_path>  (\"/Windows/System32\")
+might return C<\"/WINDOWS/system32\">  (the exact return value
+would depend on details of how the directories were originally
+created under Windows).
+
+I<Note>:
+This function does not handle drive names, backslashes etc.
+
+See also C<guestfs_realpath>.");
+
  ]

  let all_functions = non_daemon_functions @ daemon_functions
-- 1.6.5.rc2

This API seems like an unfortunate pimple. Would it not be better to *not* export this API, and instead call this automatically everywhere the daemon opens a file on ntfs?

Matt
--
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team

M:       +44 (0)7977 267231
GPG ID:  D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]