[Libguestfs] [PATCH 36/67] daemon: Move all RESOLVE macros to daemon/stubs.c.

Richard W.M. Jones rjones at redhat.com
Sat Aug 24 11:04:36 UTC 2013


From: "Richard W.M. Jones" <rjones at redhat.com>

These macros are pretty horrible to use, with unexpected side-effects.
Move them exclusively into the generated code and rewrite the one
place in the general C code which used them.

There's no functional change in this code.

(cherry picked from commit 89cf1c1163f5c01f2335b7c030bb884be3310394)
---
 daemon/daemon.h     |  93 -------------------------------------
 daemon/ext2.c       |  13 +++++-
 generator/daemon.ml | 130 +++++++++++++++++++++++++++++++++++++++++++++-------
 3 files changed, 125 insertions(+), 111 deletions(-)

diff --git a/daemon/daemon.h b/daemon/daemon.h
index 4887d21..d3a0d01 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -338,99 +338,6 @@ is_zero (const char *buffer, size_t size)
     }									\
   } while (0)
 
-/* All functions that need an argument that is a device or partition name
- * must call this macro.  It checks that the device exists and does
- * device name translation (described in the guestfs(3) manpage).
- * Note that the "path" argument may be modified.
- *
- * NB. Cannot be used for FileIn functions.
- */
-#define RESOLVE_DEVICE(path,cancel_stmt,fail_stmt)                      \
-  do {									\
-    if (STRNEQLEN ((path), "/dev/", 5)) {				\
-      cancel_stmt;                                                      \
-      reply_with_error ("%s: %s: expecting a device name", __func__, (path)); \
-      fail_stmt;							\
-    }									\
-    if (is_root_device (path)) {                                        \
-      cancel_stmt;                                                      \
-      reply_with_error ("%s: %s: device not found", __func__, path);    \
-      fail_stmt;                                                        \
-    }                                                                   \
-    if (device_name_translation ((path)) == -1) {                       \
-      int err = errno;                                                  \
-      cancel_stmt;                                                      \
-      errno = err;                                                      \
-      reply_with_perror ("%s: %s", __func__, path);                     \
-      fail_stmt;							\
-    }                                                                   \
-  } while (0)
-
-/* All functions that take a mountable argument must call this macro.
- * It parses the mountable into a mountable_t, ensures any
- * underlying device exists, and does device name translation
- * (described in the guestfs(3) manpage).
- *
- * Note that the "string" argument may be modified.
- */
-#define RESOLVE_MOUNTABLE(string,mountable,cancel_stmt,fail_stmt)       \
-  do {                                                                  \
-    if (STRPREFIX ((string), "btrfsvol:")) {                            \
-      if (parse_btrfsvol ((string) + strlen ("btrfsvol:"), &(mountable)) == -1)\
-      {                                                                 \
-        cancel_stmt;                                                    \
-        reply_with_error ("%s: %s: expecting a btrfs volume",           \
-                          __func__, (string));                          \
-        fail_stmt;                                                      \
-      }                                                                 \
-    }                                                                   \
-                                                                        \
-    else {                                                              \
-      (mountable).type = MOUNTABLE_DEVICE;                              \
-      (mountable).device = (string);                                    \
-      (mountable).volume = NULL;                                        \
-      RESOLVE_DEVICE((string), cancel_stmt, fail_stmt);                 \
-    }                                                                   \
-  } while (0)
-
-/* Helper for functions which need either an absolute path in the
- * mounted filesystem, OR a /dev/ device which exists.
- *
- * NB. Cannot be used for FileIn functions.
- *
- * NB #2: Functions which mix filenames and device paths should be
- * avoided, and existing functions should be deprecated.  This is
- * because we intend in future to make device parameters a distinct
- * type from filenames.
- */
-#define REQUIRE_ROOT_OR_RESOLVE_DEVICE(path,cancel_stmt,fail_stmt)      \
-  do {									\
-    if (STREQLEN ((path), "/dev/", 5))                                  \
-      RESOLVE_DEVICE ((path), cancel_stmt, fail_stmt);                  \
-    else {								\
-      NEED_ROOT (cancel_stmt, fail_stmt);                               \
-      ABS_PATH ((path), cancel_stmt, fail_stmt);                        \
-    }									\
-  } while (0)
-
-/* Helper for functions which need either an absolute path in the
- * mounted filesystem, OR a valid mountable description.
- */
-#define REQUIRE_ROOT_OR_RESOLVE_MOUNTABLE(string, mountable,            \
-                                          cancel_stmt, fail_stmt)       \
-  do {                                                                  \
-    if (STREQLEN ((string), "/dev/", strlen ("/dev/")) || (string)[0] != '/') {\
-      RESOLVE_MOUNTABLE (string, mountable, cancel_stmt, fail_stmt);    \
-    }                                                                   \
-                                                                        \
-    else {                                                              \
-      NEED_ROOT (cancel_stmt, fail_stmt);                               \
-      (mountable).type = MOUNTABLE_PATH;                                \
-      (mountable).device = (string);                                    \
-    }                                                                   \
-  } while (0)                                                           \
-
-
 /* NB:
  * (1) You must match CHROOT_IN and CHROOT_OUT even along error paths.
  * (2) You must not change directory!  cwd must always be "/", otherwise
diff --git a/daemon/ext2.c b/daemon/ext2.c
index ff785b7..d1ce020 100644
--- a/daemon/ext2.c
+++ b/daemon/ext2.c
@@ -946,6 +946,17 @@ do_mke2fs (const char *device,               /* 0 */
        * have to do it manually here, but note that LABEL=.. and
        * UUID=.. are valid strings which do not require translation.
        */
+      if (STRPREFIX (journaldevice, "/dev/")) {
+        if (is_root_device (journaldevice)) {
+          reply_with_error ("%s: device not found", journaldevice);
+          return -1;
+        }
+        if (device_name_translation ((char *) journaldevice) == -1) {
+          reply_with_perror ("%s", journaldevice);
+          return -1;
+        }
+      }
+
       journaldevice_s = malloc (strlen (journaldevice) + 8);
       if (!journaldevice_s) {
         reply_with_perror ("malloc");
@@ -953,8 +964,6 @@ do_mke2fs (const char *device,               /* 0 */
       }
 
       sprintf (journaldevice_s, "device=%s", journaldevice);
-      if (STRPREFIX (&journaldevice_s[7], "/dev/"))
-        RESOLVE_DEVICE (&journaldevice_s[7], , return -1);
 
       ADD_ARG (argv, i, "-J");
       ADD_ARG (argv, i, journaldevice_s);
diff --git a/generator/daemon.ml b/generator/daemon.ml
index e68622b..b08a9c2 100644
--- a/generator/daemon.ml
+++ b/generator/daemon.ml
@@ -74,22 +74,120 @@ let generate_daemon_actions_h () =
 and generate_daemon_actions () =
   generate_header CStyle GPLv2plus;
 
-  pr "#include <config.h>\n";
-  pr "\n";
-  pr "#include <stdio.h>\n";
-  pr "#include <stdlib.h>\n";
-  pr "#include <string.h>\n";
-  pr "#include <inttypes.h>\n";
-  pr "#include <errno.h>\n";
-  pr "#include <rpc/types.h>\n";
-  pr "#include <rpc/xdr.h>\n";
-  pr "\n";
-  pr "#include \"daemon.h\"\n";
-  pr "#include \"c-ctype.h\"\n";
-  pr "#include \"guestfs_protocol.h\"\n";
-  pr "#include \"actions.h\"\n";
-  pr "#include \"optgroups.h\"\n";
-  pr "\n";
+  pr "\
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <inttypes.h>
+#include <errno.h>
+#include <rpc/types.h>
+#include <rpc/xdr.h>
+
+#include \"daemon.h\"
+#include \"c-ctype.h\"
+#include \"guestfs_protocol.h\"
+#include \"actions.h\"
+#include \"optgroups.h\"
+
+/* Some macros to make resolving devices easier.  These used to
+ * be available in daemon.h but now they are only used by stubs.
+ */
+
+/* All functions that need an argument that is a device or partition name
+ * must call this macro.  It checks that the device exists and does
+ * device name translation (described in the guestfs(3) manpage).
+ * Note that the \"path\" argument may be modified.
+ *
+ * NB. Cannot be used for FileIn functions.
+ */
+#define RESOLVE_DEVICE(path,cancel_stmt,fail_stmt)                      \\
+  do {									\\
+    if (STRNEQLEN ((path), \"/dev/\", 5)) {				\\
+      cancel_stmt;                                                      \\
+      reply_with_error (\"%%s: %%s: expecting a device name\", __func__, (path)); \\
+      fail_stmt;							\\
+    }									\\
+    if (is_root_device (path)) {                                        \\
+      cancel_stmt;                                                      \\
+      reply_with_error (\"%%s: %%s: device not found\", __func__, path);    \\
+      fail_stmt;                                                        \\
+    }                                                                   \\
+    if (device_name_translation ((path)) == -1) {                       \\
+      int err = errno;                                                  \\
+      cancel_stmt;                                                      \\
+      errno = err;                                                      \\
+      reply_with_perror (\"%%s: %%s\", __func__, path);                     \\
+      fail_stmt;							\\
+    }                                                                   \\
+  } while (0)
+
+/* All functions that take a mountable argument must call this macro.
+ * It parses the mountable into a mountable_t, ensures any
+ * underlying device exists, and does device name translation
+ * (described in the guestfs(3) manpage).
+ *
+ * Note that the \"string\" argument may be modified.
+ */
+#define RESOLVE_MOUNTABLE(string,mountable,cancel_stmt,fail_stmt)       \\
+  do {                                                                  \\
+    if (STRPREFIX ((string), \"btrfsvol:\")) {                            \\
+      if (parse_btrfsvol ((string) + strlen (\"btrfsvol:\"), &(mountable)) == -1)\\
+      {                                                                 \\
+        cancel_stmt;                                                    \\
+        reply_with_error (\"%%s: %%s: expecting a btrfs volume\",           \\
+                          __func__, (string));                          \\
+        fail_stmt;                                                      \\
+      }                                                                 \\
+    }                                                                   \\
+                                                                        \\
+    else {                                                              \\
+      (mountable).type = MOUNTABLE_DEVICE;                              \\
+      (mountable).device = (string);                                    \\
+      (mountable).volume = NULL;                                        \\
+      RESOLVE_DEVICE((string), cancel_stmt, fail_stmt);                 \\
+    }                                                                   \\
+  } while (0)
+
+/* Helper for functions which need either an absolute path in the
+ * mounted filesystem, OR a /dev/ device which exists.
+ *
+ * NB. Cannot be used for FileIn functions.
+ *
+ * NB #2: Functions which mix filenames and device paths should be
+ * avoided, and existing functions should be deprecated.  This is
+ * because we intend in future to make device parameters a distinct
+ * type from filenames.
+ */
+#define REQUIRE_ROOT_OR_RESOLVE_DEVICE(path,cancel_stmt,fail_stmt)      \\
+  do {									\\
+    if (STREQLEN ((path), \"/dev/\", 5))                                  \\
+      RESOLVE_DEVICE ((path), cancel_stmt, fail_stmt);                  \\
+    else {								\\
+      NEED_ROOT (cancel_stmt, fail_stmt);                               \\
+      ABS_PATH ((path), cancel_stmt, fail_stmt);                        \\
+    }									\\
+  } while (0)
+
+/* Helper for functions which need either an absolute path in the
+ * mounted filesystem, OR a valid mountable description.
+ */
+#define REQUIRE_ROOT_OR_RESOLVE_MOUNTABLE(string, mountable,            \\
+                                          cancel_stmt, fail_stmt)       \\
+  do {                                                                  \\
+    if (STREQLEN ((string), \"/dev/\", strlen (\"/dev/\")) || (string)[0] != '/') {\\
+      RESOLVE_MOUNTABLE (string, mountable, cancel_stmt, fail_stmt);    \\
+    }                                                                   \\
+                                                                        \\
+    else {                                                              \\
+      NEED_ROOT (cancel_stmt, fail_stmt);                               \\
+      (mountable).type = MOUNTABLE_PATH;                                \\
+      (mountable).device = (string);                                    \\
+    }                                                                   \\
+  } while (0)                                                           \\
+
+";
 
   List.iter (
     fun { name = name; style = ret, args, optargs; optional = optional } ->
-- 
1.8.3.1




More information about the Libguestfs mailing list