[Qemu-devel] [PATCH] util: Emancipate id_wellformed() from QemuOpts

[Markus Armbruster]

IDs have long spread beyond QemuOpts: not everything with an ID
necessarily goes through QemuOpts.  Commit 9aebf3b is about such a
case: block layer names are meant to be well-formed IDs, but some of
them don't go through QemuOpts, and thus weren't checked.  The commit
fixed that the straightforward way: rename the internal QemuOpts
helper id_wellformed() to qemu_opts_id_wellformed() and give it
external linkage.

Instead of using it directly in block.c, the commit adds wrapper
bdrv_is_valid_name(), probably to hide the connection to QemuOpts.

Go one logical step further: emancipate IDs from QemuOpts.  Rename the
function back to id_wellformed(), and put it in another file.  While
there, clean up its value to bool.  Peel off the bdrv_is_valid_name()
wrapper.
---
 block.c               |  9 ++-------
 include/qemu-common.h |  3 +++
 include/qemu/option.h |  1 -
 util/Makefile.objs    |  1 +
 util/id.c             | 28 ++++++++++++++++++++++++++++
 util/qemu-option.c    | 17 +----------------
 6 files changed, 35 insertions(+), 24 deletions(-)
 create mode 100644 util/id.c

diff --git a/block.c b/block.c
index c5a251c..d3aebeb 100644
--- a/block.c
+++ b/block.c
@@ -335,18 +335,13 @@ void bdrv_register(BlockDriver *bdrv)
     QLIST_INSERT_HEAD(&bdrv_drivers, bdrv, list);
 }
 
-static bool bdrv_is_valid_name(const char *name)
-{
-    return qemu_opts_id_wellformed(name);
-}
-
 /* create a new block device (by default it is empty) */
 BlockDriverState *bdrv_new(const char *device_name, Error **errp)
 {
     BlockDriverState *bs;
     int i;
 
-    if (*device_name && !bdrv_is_valid_name(device_name)) {
+    if (*device_name && !id_wellformed(device_name)) {
         error_setg(errp, "Invalid device name");
         return NULL;
     }
@@ -874,7 +869,7 @@ static void bdrv_assign_node_name(BlockDriverState *bs,
     }
 
     /* Check for empty string or invalid characters */
-    if (!bdrv_is_valid_name(node_name)) {
+    if (!id_wellformed(node_name)) {
         error_setg(errp, "Invalid node name");
         return;
     }
diff --git a/include/qemu-common.h b/include/qemu-common.h
index dcb57ab..b87e9c2 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -190,6 +190,9 @@ int64_t strtosz_suffix_unit(const char *nptr, char **end,
 /* used to print char* safely */
 #define STR_OR_NULL(str) ((str) ? (str) : "null")
 
+/* id.c */
+bool id_wellformed(const char *id);
+
 /* path.c */
 void init_paths(const char *prefix);
 const char *path(const char *pathname);
diff --git a/include/qemu/option.h b/include/qemu/option.h
index 945347c..59bea75 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -103,7 +103,6 @@ typedef int (*qemu_opt_loopfunc)(const char *name, const 
char *value, void *opaq
 int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque,
                      int abort_on_failure);
 
-int qemu_opts_id_wellformed(const char *id);
 QemuOpts *qemu_opts_find(QemuOptsList *list, const char *id);
 QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id,
                            int fail_if_exists, Error **errp);
diff --git a/util/Makefile.objs b/util/Makefile.objs
index cb8862b..93007e2 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -8,6 +8,7 @@ util-obj-y += fifo8.o
 util-obj-y += acl.o
 util-obj-y += error.o qemu-error.o
 util-obj-$(CONFIG_POSIX) += compatfd.o
+util-obj-y += id.o
 util-obj-y += iov.o aes.o qemu-config.o qemu-sockets.o uri.o notify.o
 util-obj-y += qemu-option.o qemu-progress.o
 util-obj-y += hexdump.o
diff --git a/util/id.c b/util/id.c
new file mode 100644
index 0000000..0f2c42a
--- /dev/null
+++ b/util/id.c
@@ -0,0 +1,28 @@
+/*
+ * Dealing with identifiers
+ *
+ * Copyright (C) 2014 Red Hat, Inc.
+ *
+ * Authors:
+ *  Markus Armbruster <address@hidden>,
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1
+ * or later.  See the COPYING.LIB file in the top-level directory.
+ */
+
+#include "qemu-common.h"
+
+bool id_wellformed(const char *id)
+{
+    int i;
+
+    if (!qemu_isalpha(id[0])) {
+        return 0;
+    }
+    for (i = 1; id[i]; i++) {
+        if (!qemu_isalnum(id[i]) && !strchr("-._", id[i])) {
+            return false;
+        }
+    }
+    return true;
+}
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 0cf9960..5d10695 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -641,28 +641,13 @@ QemuOpts *qemu_opts_find(QemuOptsList *list, const char 
*id)
     return NULL;
 }
 
-int qemu_opts_id_wellformed(const char *id)
-{
-    int i;
-
-    if (!qemu_isalpha(id[0])) {
-        return 0;
-    }
-    for (i = 1; id[i]; i++) {
-        if (!qemu_isalnum(id[i]) && !strchr("-._", id[i])) {
-            return 0;
-        }
-    }
-    return 1;
-}
-
 QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id,
                            int fail_if_exists, Error **errp)
 {
     QemuOpts *opts = NULL;
 
     if (id) {
-        if (!qemu_opts_id_wellformed(id)) {
+        if (!id_wellformed(id)) {
             error_set(errp,QERR_INVALID_PARAMETER_VALUE, "id", "an 
identifier");
 #if 0 /* conversion from qerror_report() to error_set() broke this: */
             error_printf_unless_qmp("Identifiers consist of letters, digits, 
'-', '.', '_', starting with a letter.\n");
-- 
1.9.3