diff mbox

[v23,09/32] add qemu_opts_append to repalce append_option_parameters

Message ID 1395396763-26081-10-git-send-email-cyliu@suse.com
State New
Headers show

Commit Message

Chunyan Liu March 21, 2014, 10:12 a.m. UTC
For later merge .create_opts of drv and proto_drv in qemu-img commands.

Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
Changes:
  * Following Eric's suggestion, qemu_opts_append() will accept a pair of
    parameters (QEMUOptionParameter and QemuOpts), to handle the inconsistency
    that some driver uses QemuOpts while some driver uses QEMUOptionParameter.
  * using g_realloc of first parameter 'dst' and return it, instead of malloc
    a new list and copy data from all parameters to the new list, and return
    the new malloced list. Solving memory free effort for multiple append(s),
    and no naming problem as in previous version.

 include/qemu/option.h |  5 ++++
 util/qemu-option.c    | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+)

Comments

Leandro Dorileo March 25, 2014, 7:13 p.m. UTC | #1
On Fri, Mar 21, 2014 at 06:12:20PM +0800, Chunyan Liu wrote:
> For later merge .create_opts of drv and proto_drv in qemu-img commands.
> 
> Signed-off-by: Chunyan Liu <cyliu@suse.com>

Reviewed-by: Leandro Dorileo <l@dorileo.org>


> ---
> Changes:
>   * Following Eric's suggestion, qemu_opts_append() will accept a pair of
>     parameters (QEMUOptionParameter and QemuOpts), to handle the inconsistency
>     that some driver uses QemuOpts while some driver uses QEMUOptionParameter.
>   * using g_realloc of first parameter 'dst' and return it, instead of malloc
>     a new list and copy data from all parameters to the new list, and return
>     the new malloced list. Solving memory free effort for multiple append(s),
>     and no naming problem as in previous version.
> 
>  include/qemu/option.h |  5 ++++
>  util/qemu-option.c    | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 70 insertions(+)
> 
> diff --git a/include/qemu/option.h b/include/qemu/option.h
> index fd6f075..120c998 100644
> --- a/include/qemu/option.h
> +++ b/include/qemu/option.h
> @@ -175,5 +175,10 @@ void qemu_opts_print_help(QemuOptsList *list);
>  void qemu_opts_free(QemuOptsList *list);
>  QEMUOptionParameter *opts_to_params(QemuOpts *opts);
>  QemuOptsList *params_to_opts(QEMUOptionParameter *list);
> +/* FIXME: will remove QEMUOptionParameter after all drivers switch to QemuOpts.
> + */
> +QemuOptsList *qemu_opts_append(QemuOptsList *dst,
> +                               QemuOptsList *list,
> +                               QEMUOptionParameter *param);
>  
>  #endif
> diff --git a/util/qemu-option.c b/util/qemu-option.c
> index 4bdcfbf..6c304b2 100644
> --- a/util/qemu-option.c
> +++ b/util/qemu-option.c
> @@ -1477,3 +1477,68 @@ void qemu_opts_free(QemuOptsList *list)
>  
>      g_free(list);
>  }
> +
> +/* Realloc dst option list and append options either from an option list (list)
> + * or an QEMUOptionParameter (param) to it. dst could be NULL or a malloced list.
> + * FIXME: will remove QEMUOptionParameter after all drivers switch to QemuOpts.
> + */
> +QemuOptsList *qemu_opts_append(QemuOptsList *dst,
> +                               QemuOptsList *list,
> +                               QEMUOptionParameter *param)
> +{
> +    size_t num_opts, num_dst_opts;
> +    QemuOptsList *tmp_list = NULL;
> +    QemuOptDesc *desc;
> +    bool need_init = false;
> +
> +    assert(!(list && param));
> +    if (!param &&!list) {
> +        return dst;
> +    }
> +
> +    if (param) {
> +        list = tmp_list = params_to_opts(param);
> +    }
> +
> +    /* If dst is NULL, after realloc, some area of dst should be initialized
> +     * before adding options to it.
> +     */
> +    if (!dst) {
> +        need_init = true;
> +    }
> +
> +    num_opts = count_opts_list(dst);
> +    num_dst_opts = num_opts;
> +    num_opts += count_opts_list(list);
> +    dst = g_realloc(dst, sizeof(QemuOptsList) +
> +                    (num_opts + 1) * sizeof(QemuOptDesc));
> +    if (need_init) {
> +        dst->name = NULL;
> +        dst->implied_opt_name = NULL;
> +        QTAILQ_INIT(&dst->head);
> +        dst->mallocd = true;
> +    }
> +    dst->desc[num_dst_opts].name = NULL;
> +
> +    /* (const char *) members of result dst are malloced, need free. */
> +    assert(dst->mallocd);
> +    /* append list->desc to dst->desc */
> +    if (list) {
> +        desc = list->desc;
> +        while (desc && desc->name) {
> +            if (find_desc_by_name(dst->desc, desc->name) == NULL) {
> +                dst->desc[num_dst_opts].name = g_strdup(desc->name);
> +                dst->desc[num_dst_opts].type = desc->type;
> +                dst->desc[num_dst_opts].help = g_strdup(desc->help);
> +                dst->desc[num_dst_opts].def_value_str =
> +                                         g_strdup(desc->def_value_str);
> +                num_dst_opts++;
> +                dst->desc[num_dst_opts].name = NULL;
> +            }
> +            desc++;
> +        }
> +    }
> +
> +    g_free(tmp_list);
> +    return dst;
> +}
> -- 
> 1.7.12.4
> 
>
Eric Blake March 25, 2014, 9:40 p.m. UTC | #2
On 03/21/2014 04:12 AM, Chunyan Liu wrote:

s/repalce/replace/ in subject

Also, your overall series could probably use subject lines with a prefix
that makes it obvious they are related to a common category; maybe "opts: ".

> For later merge .create_opts of drv and proto_drv in qemu-img commands.
> 
> Signed-off-by: Chunyan Liu <cyliu@suse.com>
> ---

> +
> +/* Realloc dst option list and append options either from an option list (list)
> + * or an QEMUOptionParameter (param) to it. dst could be NULL or a malloced list.

s/an QEMU/a QEMU/

> + * FIXME: will remove QEMUOptionParameter after all drivers switch to QemuOpts.
> + */
> +QemuOptsList *qemu_opts_append(QemuOptsList *dst,
> +                               QemuOptsList *list,
> +                               QEMUOptionParameter *param)
> +{
> +    size_t num_opts, num_dst_opts;
> +    QemuOptsList *tmp_list = NULL;
> +    QemuOptDesc *desc;
> +    bool need_init = false;
> +
> +    assert(!(list && param));
> +    if (!param &&!list) {

Space after &&
diff mbox

Patch

diff --git a/include/qemu/option.h b/include/qemu/option.h
index fd6f075..120c998 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -175,5 +175,10 @@  void qemu_opts_print_help(QemuOptsList *list);
 void qemu_opts_free(QemuOptsList *list);
 QEMUOptionParameter *opts_to_params(QemuOpts *opts);
 QemuOptsList *params_to_opts(QEMUOptionParameter *list);
+/* FIXME: will remove QEMUOptionParameter after all drivers switch to QemuOpts.
+ */
+QemuOptsList *qemu_opts_append(QemuOptsList *dst,
+                               QemuOptsList *list,
+                               QEMUOptionParameter *param);
 
 #endif
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 4bdcfbf..6c304b2 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -1477,3 +1477,68 @@  void qemu_opts_free(QemuOptsList *list)
 
     g_free(list);
 }
+
+/* Realloc dst option list and append options either from an option list (list)
+ * or an QEMUOptionParameter (param) to it. dst could be NULL or a malloced list.
+ * FIXME: will remove QEMUOptionParameter after all drivers switch to QemuOpts.
+ */
+QemuOptsList *qemu_opts_append(QemuOptsList *dst,
+                               QemuOptsList *list,
+                               QEMUOptionParameter *param)
+{
+    size_t num_opts, num_dst_opts;
+    QemuOptsList *tmp_list = NULL;
+    QemuOptDesc *desc;
+    bool need_init = false;
+
+    assert(!(list && param));
+    if (!param &&!list) {
+        return dst;
+    }
+
+    if (param) {
+        list = tmp_list = params_to_opts(param);
+    }
+
+    /* If dst is NULL, after realloc, some area of dst should be initialized
+     * before adding options to it.
+     */
+    if (!dst) {
+        need_init = true;
+    }
+
+    num_opts = count_opts_list(dst);
+    num_dst_opts = num_opts;
+    num_opts += count_opts_list(list);
+    dst = g_realloc(dst, sizeof(QemuOptsList) +
+                    (num_opts + 1) * sizeof(QemuOptDesc));
+    if (need_init) {
+        dst->name = NULL;
+        dst->implied_opt_name = NULL;
+        QTAILQ_INIT(&dst->head);
+        dst->mallocd = true;
+    }
+    dst->desc[num_dst_opts].name = NULL;
+
+    /* (const char *) members of result dst are malloced, need free. */
+    assert(dst->mallocd);
+    /* append list->desc to dst->desc */
+    if (list) {
+        desc = list->desc;
+        while (desc && desc->name) {
+            if (find_desc_by_name(dst->desc, desc->name) == NULL) {
+                dst->desc[num_dst_opts].name = g_strdup(desc->name);
+                dst->desc[num_dst_opts].type = desc->type;
+                dst->desc[num_dst_opts].help = g_strdup(desc->help);
+                dst->desc[num_dst_opts].def_value_str =
+                                         g_strdup(desc->def_value_str);
+                num_dst_opts++;
+                dst->desc[num_dst_opts].name = NULL;
+            }
+            desc++;
+        }
+    }
+
+    g_free(tmp_list);
+    return dst;
+}