acpi_table_add(): accept QemuOpts and parse it with OptsVisitor

As one consequence, strtok() -- which modifies its argument -- is replaced
with g_strsplit().

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Message-id: 1363821803-3380-6-git-send-email-lersek@redhat.com
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

arch_init.c

@@ -1103,10 +1103,10 @@ int qemu_uuid_parse(const char *str, uint8_t *uuid)
     return 0;
 }
 
-void do_acpitable_option(const char *optarg)
+void do_acpitable_option(const QemuOpts *opts)
 {
 #ifdef TARGET_I386
-    if (acpi_table_add(optarg) < 0) {
+    if (acpi_table_add(opts) < 0) {
         fprintf(stderr, "Wrong acpi table provided\n");
         exit(1);
     }

hw/acpi.c

@@ -23,6 +23,10 @@
 #include "hw/pc.h"
 #include "hw/acpi.h"
 #include "monitor/monitor.h"
+#include "qemu/config-file.h"
+#include "qapi/opts-visitor.h"
+#include "qapi/dealloc-visitor.h"
+#include "qapi-visit.h"
 
 struct acpi_table_header {
     uint16_t _length;         /* our length, not actual part of the hdr */
@@ -51,6 +55,20 @@ static const char unsigned dfl_hdr[ACPI_TABLE_HDR_SIZE] =
 char unsigned *acpi_tables;
 size_t acpi_tables_len;
 
+static QemuOptsList qemu_acpi_opts = {
+    .name = "acpi",
+    .implied_opt_name = "data",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_acpi_opts.head),
+    .desc = { { 0 } } /* validated with OptsVisitor */
+};
+
+static void acpi_register_config(void)
+{
+    qemu_add_opts(&qemu_acpi_opts);
+}
+
+machine_init(acpi_register_config);
+
 static int acpi_checksum(const uint8_t *data, int len)
 {
     int sum, i;
@@ -61,12 +79,13 @@ static int acpi_checksum(const uint8_t *data, int len)
     return (-sum) & 0xff;
 }
 
-/* XXX fixme: this function uses obsolete argument parsing interface */
+int acpi_table_add(const QemuOpts *opts)
-int acpi_table_add(const char *t)
 {
+    AcpiTableOptions *hdrs = NULL;
     Error *err = NULL;
-    char buf[1024], *p, *f;
+    char **pathnames = NULL;
-    unsigned long val;
+    char **cur;
+
     size_t len, start, allen;
     bool has_header;
     int changed;
@@ -74,21 +93,26 @@ int acpi_table_add(const char *t)
     struct acpi_table_header hdr;
     char unsigned *table_start;
 
-    r = 0;
+    {
-    r |= get_param_value(buf, sizeof(buf), "data", t) ? 1 : 0;
+        OptsVisitor *ov;
-    r |= get_param_value(buf, sizeof(buf), "file", t) ? 2 : 0;
+
-    switch (r) {
+        ov = opts_visitor_new(opts);
-    case 0:
+        visit_type_AcpiTableOptions(opts_get_visitor(ov), &hdrs, NULL, &err);
-        buf[0] = '\0';
+        opts_visitor_cleanup(ov);
-        /* fallthrough for default behavior */
+    }
-    case 1:
+
-        has_header = false;
+    if (err) {
-        break;
+        goto out;
-    case 2:
+    }
-        has_header = true;
+    if (hdrs->has_file == hdrs->has_data) {
-        break;
+        error_setg(&err, "'-acpitable' requires one of 'data' or 'file'");
-    default:
+        goto out;
-        error_setg(&err, "acpitable: both data and file are specified");
+    }
+    has_header = hdrs->has_file;
+
+    pathnames = g_strsplit(hdrs->has_file ? hdrs->file : hdrs->data, ":", 0);
+    if (pathnames == NULL || pathnames[0] == NULL) {
+        error_setg(&err, "'-acpitable' requires at least one pathname");
         goto out;
     }
 
@@ -105,11 +129,11 @@ int acpi_table_add(const char *t)
 
     /* now read in the data files, reallocating buffer as needed */
 
-    for (f = strtok(buf, ":"); f; f = strtok(NULL, ":")) {
+    for (cur = pathnames; *cur; ++cur) {
-        int fd = open(f, O_RDONLY | O_BINARY);
+        int fd = open(*cur, O_RDONLY | O_BINARY);
 
         if (fd < 0) {
-            error_setg(&err, "can't open file %s: %s", f, strerror(errno));
+            error_setg(&err, "can't open file %s: %s", *cur, strerror(errno));
             goto out;
         }
 
@@ -124,7 +148,7 @@ int acpi_table_add(const char *t)
                 allen += r;
             } else if (errno != EINTR) {
                 error_setg(&err, "can't read file %s: %s",
-                           f, strerror(errno));
+                           *cur, strerror(errno));
                 close(fd);
                 goto out;
             }
@@ -146,14 +170,16 @@ int acpi_table_add(const char *t)
 
     hdr._length = cpu_to_le16(len);
 
-    if (get_param_value(buf, sizeof(buf), "sig", t)) {
+    if (hdrs->has_sig) {
         /* strncpy is justified: the field need not be NUL-terminated. */
-        strncpy(hdr.sig, buf, sizeof(hdr.sig));
+        strncpy(hdr.sig, hdrs->sig, sizeof(hdr.sig));
         ++changed;
     }
 
     /* length of the table including header, in bytes */
     if (has_header) {
+        unsigned long val;
+
         /* check if actual length is correct */
         val = le32_to_cpu(hdr.length);
         if (val != len) {
@@ -167,52 +193,38 @@ int acpi_table_add(const char *t)
     /* we may avoid putting length here if has_header is true */
     hdr.length = cpu_to_le32(len);
 
-    if (get_param_value(buf, sizeof(buf), "rev", t)) {
+    if (hdrs->has_rev) {
-        val = strtoul(buf, &p, 0);
+        hdr.revision = hdrs->rev;
-        if (val > 255 || *p) {
-            error_setg(&err, "acpitable: \"rev=%s\" is invalid", buf);
-            goto out;
-        }
-        hdr.revision = (uint8_t)val;
         ++changed;
     }
 
-    if (get_param_value(buf, sizeof(buf), "oem_id", t)) {
+    if (hdrs->has_oem_id) {
         /* strncpy is justified: the field need not be NUL-terminated. */
-        strncpy(hdr.oem_id, buf, sizeof(hdr.oem_id));
+        strncpy(hdr.oem_id, hdrs->oem_id, sizeof(hdr.oem_id));
         ++changed;
     }
 
-    if (get_param_value(buf, sizeof(buf), "oem_table_id", t)) {
+    if (hdrs->has_oem_table_id) {
         /* strncpy is justified: the field need not be NUL-terminated. */
-        strncpy(hdr.oem_table_id, buf, sizeof(hdr.oem_table_id));
+        strncpy(hdr.oem_table_id, hdrs->oem_table_id,
+                sizeof(hdr.oem_table_id));
         ++changed;
     }
 
-    if (get_param_value(buf, sizeof(buf), "oem_rev", t)) {
+    if (hdrs->has_oem_rev) {
-        val = strtol(buf, &p, 0);
+        hdr.oem_revision = cpu_to_le32(hdrs->oem_rev);
-        if (*p) {
-            error_setg(&err, "acpitable: \"oem_rev=%s\" is invalid", buf);
-            goto out;
-        }
-        hdr.oem_revision = cpu_to_le32(val);
         ++changed;
     }
 
-    if (get_param_value(buf, sizeof(buf), "asl_compiler_id", t)) {
+    if (hdrs->has_asl_compiler_id) {
         /* strncpy is justified: the field need not be NUL-terminated. */
-        strncpy(hdr.asl_compiler_id, buf, sizeof(hdr.asl_compiler_id));
+        strncpy(hdr.asl_compiler_id, hdrs->asl_compiler_id,
+                sizeof(hdr.asl_compiler_id));
         ++changed;
     }
 
-    if (get_param_value(buf, sizeof(buf), "asl_compiler_rev", t)) {
+    if (hdrs->has_asl_compiler_rev) {
-        val = strtol(buf, &p, 0);
+        hdr.asl_compiler_revision = cpu_to_le32(hdrs->asl_compiler_rev);
-        if (*p) {
-            error_setg(&err, "acpitable: \"%s=%s\" is invalid",
-                       "asl_compiler_rev", buf);
-            goto out;
-        }
-        hdr.asl_compiler_revision = cpu_to_le32(val);
         ++changed;
     }
 
@@ -239,12 +251,25 @@ int acpi_table_add(const char *t)
         cpu_to_le32(le32_to_cpu(*(uint16_t *)acpi_tables) + 1);
 
     acpi_tables_len = allen;
-    return 0;
 
 out:
+    g_strfreev(pathnames);
+
+    if (hdrs != NULL) {
+        QapiDeallocVisitor *dv;
+
+        dv = qapi_dealloc_visitor_new();
+        visit_type_AcpiTableOptions(qapi_dealloc_get_visitor(dv), &hdrs, NULL,
+                                    NULL);
+        qapi_dealloc_visitor_cleanup(dv);
+    }
+
+    if (err) {
         fprintf(stderr, "%s\n", error_get_pretty(err));
         error_free(err);
         return -1;
+    }
+    return 0;
 }
 
 static void acpi_notify_wakeup(Notifier *notifier, void *data)

hw/i386/pc.c

@@ -51,6 +51,7 @@
 #include "exec/address-spaces.h"
 #include "sysemu/arch_init.h"
 #include "qemu/bitmap.h"
+#include "qemu/config-file.h"
 
 /* debug PC/ISA interrupts */
 //#define DEBUG_IRQ
@@ -889,6 +890,7 @@ void pc_cpus_init(const char *cpu_model)
 void pc_acpi_init(const char *default_dsdt)
 {
     char *filename = NULL, *arg = NULL;
+    QemuOpts *opts;
 
     if (acpi_tables != NULL) {
         /* manually set via -acpitable, leave it alone */
@@ -902,7 +904,12 @@ void pc_acpi_init(const char *default_dsdt)
     }
 
     arg = g_strdup_printf("file=%s", filename);
-    if (acpi_table_add(arg) != 0) {
+
+    /* creates a deep copy of "arg" */
+    opts = qemu_opts_parse(qemu_find_opts("acpi"), arg, 0);
+    g_assert(opts != NULL);
+
+    if (acpi_table_add(opts) != 0) {
         fprintf(stderr, "WARNING: failed to load %s\n", filename);
     }
     g_free(arg);

hw/pc.h

@@ -113,7 +113,7 @@ extern char unsigned *acpi_tables;
 extern size_t acpi_tables_len;
 
 void acpi_bios_init(void);
-int acpi_table_add(const char *table_desc);
+int acpi_table_add(const QemuOpts *opts);
 
 /* acpi_piix.c */
 

include/sysemu/arch_init.h

@@ -2,6 +2,7 @@
 #define QEMU_ARCH_INIT_H
 
 #include "qmp-commands.h"
+#include "qemu/option.h"
 
 enum {
     QEMU_ARCH_ALL = -1,
@@ -26,7 +27,7 @@ enum {
 extern const uint32_t arch_type;
 
 void select_soundhw(const char *optarg);
-void do_acpitable_option(const char *optarg);
+void do_acpitable_option(const QemuOpts *opts);
 void do_smbios_option(const char *optarg);
 void cpudef_init(void);
 int audio_available(void);

vl.c

@@ -3587,7 +3587,9 @@ int main(int argc, char **argv, char **envp)
                 break;
             }
             case QEMU_OPTION_acpitable:
-                do_acpitable_option(optarg);
+                opts = qemu_opts_parse(qemu_find_opts("acpi"), optarg, 1);
+                g_assert(opts != NULL);
+                do_acpitable_option(opts);
                 break;
             case QEMU_OPTION_smbios:
                 do_smbios_option(optarg);