diff mbox

[v25,31/31] QemuOpts: cleanup tmp 'allocated' member from QemuOptsList

Message ID 1397152467-17186-32-git-send-email-cyliu@suse.com
State New
Headers show

Commit Message

Chunyan Liu April 10, 2014, 5:54 p.m. UTC 
Now only qemu_opts_append uses 'allocated' to indicate free memory.
For this function only, we can also let result list's (const char *)
members point to input list's members, only if the input list has
longer lifetime than result list. In current code, that is true.
So, we can remove the 'allocated' member from QemuOptsList definition
to keep code clean.

Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
 include/qemu/option.h |  5 -----
 util/qemu-option.c    | 26 ++------------------------
 2 files changed, 2 insertions(+), 29 deletions(-)

Comments

Stefan Hajnoczi April 25, 2014, 1:43 p.m. UTC | #1 
On Fri, Apr 11, 2014 at 01:54:27AM +0800, Chunyan Liu wrote:
>  /* Realloc dst option list and append options from an option list (list)
>   * to it. dst could be NULL or a malloced list.
> + * Result dst has shorter lifetime then input list.

s/then/than/

But I think it's helpful to be more explicit:

The lifetime of dst must be shorter than the input list because the
QemuOptDesc->name, ->help, and ->def_value_str strings are shared.
Chunyan Liu April 28, 2014, 6:22 a.m. UTC | #2 
>>> On 4/25/2014 at 09:43 PM, in message
<20140425134327.GI3160@stefanha-thinkpad.redhat.com>, Stefan Hajnoczi
<stefanha@redhat.com> wrote: 
> On Fri, Apr 11, 2014 at 01:54:27AM +0800, Chunyan Liu wrote: 
> >  /* Realloc dst option list and append options from an option list (list) 
> >   * to it. dst could be NULL or a malloced list. 
> > + * Result dst has shorter lifetime then input list. 
>  
> s/then/than/ 
>  
> But I think it's helpful to be more explicit: 
>  
> The lifetime of dst must be shorter than the input list because the 
> QemuOptDesc->name, ->help, and ->def_value_str strings are shared. 
>  
>  
Will update. Thanks.

Chunyan
diff mbox

Patch

diff --git a/include/qemu/option.h b/include/qemu/option.h
index 0551beb..59bea75 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -63,11 +63,6 @@  typedef struct QemuOptDesc {
 } QemuOptDesc;
 
 struct QemuOptsList {
-    /* FIXME: Temp used for QEMUOptionParamter->QemuOpts conversion to
-     * indicate free memory. Will remove after all drivers switch to QemuOpts.
-     */
-    bool allocated;
-
     const char *name;
     const char *implied_opt_name;
     bool merge_lists;  /* Merge multiple uses of option into a single list? */
diff --git a/util/qemu-option.c b/util/qemu-option.c
index bf80b0d..9340cef 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -1076,26 +1076,12 @@  static size_t count_opts_list(QemuOptsList *list)
 
 void qemu_opts_free(QemuOptsList *list)
 {
-    /* List members point to new malloced space and need to free.
-     * FIXME:
-     * Introduced for QEMUOptionParamter->QemuOpts conversion.
-     * Will remove after all drivers switch to QemuOpts.
-     */
-    if (list && list->allocated) {
-        QemuOptDesc *desc = list->desc;
-        while (desc && desc->name) {
-            g_free((char *)desc->name);
-            g_free((char *)desc->help);
-            g_free((char *)desc->def_value_str);
-            desc++;
-        }
-    }
-
     g_free(list);
 }
 
 /* Realloc dst option list and append options from an option list (list)
  * to it. dst could be NULL or a malloced list.
+ * Result dst has shorter lifetime then input list.
  */
 QemuOptsList *qemu_opts_append(QemuOptsList *dst,
                                QemuOptsList *list)
@@ -1124,23 +1110,15 @@  QemuOptsList *qemu_opts_append(QemuOptsList *dst,
         dst->name = NULL;
         dst->implied_opt_name = NULL;
         QTAILQ_INIT(&dst->head);
-        dst->allocated = true;
     }
     dst->desc[num_dst_opts].name = NULL;
 
-    /* (const char *) members of result dst are malloced, need free. */
-    assert(dst->allocated);
     /* 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++] = *desc;
                 dst->desc[num_dst_opts].name = NULL;
             }
             desc++;