| Message ID | 1359022987-6274-3-git-send-email-wdongxu@vnet.linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
Dong Xu Wang <wdongxu@vnet.linux.ibm.com> writes: > This patch will create 4 functions, count_opts_list, append_opts_list, > free_opts_list and print_opts_list, they will used in following commits. > > Signed-off-by: Dong Xu Wang <wdongxu@vnet.linux.ibm.com> > --- > v6->v7): > 1) Fix typo. > > v5->v6): > 1) allocate enough space in append_opts_list function. > > include/qemu/option.h | 4 +++ > util/qemu-option.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 94 insertions(+) > > diff --git a/include/qemu/option.h b/include/qemu/option.h > index 394170a..f784c2e 100644 > --- a/include/qemu/option.h > +++ b/include/qemu/option.h > @@ -156,4 +156,8 @@ int qemu_opts_print(QemuOpts *opts, void *dummy); > int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void *opaque, > int abort_on_failure); > > +QemuOptsList *append_opts_list(QemuOptsList *dest, > + QemuOptsList *list); > +void free_opts_list(QemuOptsList *list); > +void print_opts_list(QemuOptsList *list); Please stick to the existing naming convention: qemu_opts_append(), qemu_opts_free(), qemu_opts_print(). > #endif > diff --git a/util/qemu-option.c b/util/qemu-option.c > index 1aed418..f4bbbf8 100644 > --- a/util/qemu-option.c > +++ b/util/qemu-option.c > @@ -1152,3 +1152,93 @@ int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void *opaque, > loc_pop(&loc); > return rc; > } > + > +static size_t count_opts_list(QemuOptsList *list) > +{ > + size_t i = 0; > + > + while (list && list->desc[i].name) { > + i++; > + } Please don't invent your own way to interate over list->desc[]! Imitate the existing code. for (i = 0; list && list->desc[i].name; i++) ; Same several times below. > + > + return i; > +} > + > +/* Create a new QemuOptsList and make its desc to the merge of first and second. > + * It will allocate space for one new QemuOptsList plus enough space for > + * QemuOptDesc in first and second QemuOptsList. First argument's QemuOptDesc > + * members take precedence over second's. > + */ Unlike most functions dealing with QemuOptsLists, this one can take null arguments. Worth mentioning. Please wrap your lines a bit earlier. Column 70 is a good limit. > +QemuOptsList *append_opts_list(QemuOptsList *first, > + QemuOptsList *second) > +{ > + size_t num_first_options, num_second_options; > + QemuOptsList *dest = NULL; > + int i = 0; > + int index = 0; > + > + num_first_options = count_opts_list(first); > + num_second_options = count_opts_list(second); > + if (num_first_options + num_second_options == 0) { > + return NULL; > + } Why do you need this extra case? Shouldn't the code below just work? > + > + dest = g_malloc0(sizeof(QemuOptsList) > + + (num_first_options + num_second_options + 1) * sizeof(QemuOptDesc)); > + > + dest->name = "append_opts_list"; > + dest->implied_opt_name = NULL; Not copied from an argument. Tolerable, because all you lose is the convenience to omit name= in one place, but worth mentioning in the function comment. > + dest->merge_lists = false; Not copied from an argument. I'm afraid the result will make no sense if either argument has it set. Consider asserting they don't, and documenting the requirement in the function comment. > + QTAILQ_INIT(&dest->head); > + while (first && (first->desc[i].name)) { > + if (!find_desc_by_name(dest->desc, first->desc[i].name)) { > + dest->desc[index].name = g_strdup(first->desc[i].name); > + dest->desc[index].help = g_strdup(first->desc[i].help); > + dest->desc[index].type = first->desc[i].type; > + dest->desc[index].def_value_str = > + g_strdup(first->desc[i].def_value_str); > + ++index; index++ please, for consistency with the similar increment two lines below. > + } > + i++; Indentation's off. > + } > + i = 0; > + while (second && (second->desc[i].name)) { > + if (!find_desc_by_name(dest->desc, second->desc[i].name)) { > + dest->desc[index].name = g_strdup(first->desc[i].name); > + dest->desc[index].help = g_strdup(first->desc[i].help); > + dest->desc[index].type = second->desc[i].type; > + dest->desc[index].def_value_str = > + g_strdup(second->desc[i].def_value_str); > + ++index; > + } > + i++; > + } We've seen this loop before. Please avoid the code duplication, as I asked you before. > + dest->desc[index].name = NULL; > + return dest; > +} > + > +void free_opts_list(QemuOptsList *list) > +{ > + int i = 0; > + > + while (list && list->desc[i].name) { > + g_free((char *)list->desc[i].name); > + g_free((char *)list->desc[i].help); > + g_free((char *)list->desc[i].def_value_str); > + i++; > + } > + > + g_free(list); > +} Unlike most functions dealing with QemuOptsLists, this one can take null arguments. Makes sense, but it's worth mentioning in a function comment. > + > +void print_opts_list(QemuOptsList *list) > +{ > + int i = 0; > + printf("Supported options:\n"); > + while (list && list->desc[i].name) { > + printf("%-16s %s\n", list->desc[i].name, > + list->desc[i].help ? > + list->desc[i].help : "No description available"); Clearer: list->desc[i].help ?: "No description available"); > + i++; > + } > +} Unlike most functions dealing with QemuOptsLists, this one can take null arguments. Later patches feed it the result of append_opts_list(), which can be null, but that makes little sense to me.
On Thu, Jan 24, 2013 at 6:59 PM, Markus Armbruster <armbru@redhat.com> wrote: > Dong Xu Wang <wdongxu@vnet.linux.ibm.com> writes: > >> This patch will create 4 functions, count_opts_list, append_opts_list, >> free_opts_list and print_opts_list, they will used in following commits. >> >> Signed-off-by: Dong Xu Wang <wdongxu@vnet.linux.ibm.com> >> --- >> v6->v7): >> 1) Fix typo. >> >> v5->v6): >> 1) allocate enough space in append_opts_list function. >> >> include/qemu/option.h | 4 +++ >> util/qemu-option.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 94 insertions(+) >> >> diff --git a/include/qemu/option.h b/include/qemu/option.h >> index 394170a..f784c2e 100644 >> --- a/include/qemu/option.h >> +++ b/include/qemu/option.h >> @@ -156,4 +156,8 @@ int qemu_opts_print(QemuOpts *opts, void *dummy); >> int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void *opaque, >> int abort_on_failure); >> >> +QemuOptsList *append_opts_list(QemuOptsList *dest, >> + QemuOptsList *list); >> +void free_opts_list(QemuOptsList *list); >> +void print_opts_list(QemuOptsList *list); > > Please stick to the existing naming convention: qemu_opts_append(), > qemu_opts_free(), qemu_opts_print(). > >> #endif >> diff --git a/util/qemu-option.c b/util/qemu-option.c >> index 1aed418..f4bbbf8 100644 >> --- a/util/qemu-option.c >> +++ b/util/qemu-option.c >> @@ -1152,3 +1152,93 @@ int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void *opaque, >> loc_pop(&loc); >> return rc; >> } >> + >> +static size_t count_opts_list(QemuOptsList *list) >> +{ >> + size_t i = 0; >> + >> + while (list && list->desc[i].name) { >> + i++; >> + } > > Please don't invent your own way to interate over list->desc[]! Imitate > the existing code. > > for (i = 0; list && list->desc[i].name; i++) ; > > Same several times below. > >> + >> + return i; >> +} >> + >> +/* Create a new QemuOptsList and make its desc to the merge of first and second. >> + * It will allocate space for one new QemuOptsList plus enough space for >> + * QemuOptDesc in first and second QemuOptsList. First argument's QemuOptDesc >> + * members take precedence over second's. >> + */ > > Unlike most functions dealing with QemuOptsLists, this one can take null > arguments. Worth mentioning. > > Please wrap your lines a bit earlier. Column 70 is a good limit. > >> +QemuOptsList *append_opts_list(QemuOptsList *first, >> + QemuOptsList *second) >> +{ >> + size_t num_first_options, num_second_options; >> + QemuOptsList *dest = NULL; >> + int i = 0; >> + int index = 0; >> + >> + num_first_options = count_opts_list(first); >> + num_second_options = count_opts_list(second); >> + if (num_first_options + num_second_options == 0) { >> + return NULL; >> + } > > Why do you need this extra case? Shouldn't the code below just work? > >> + >> + dest = g_malloc0(sizeof(QemuOptsList) >> + + (num_first_options + num_second_options + 1) * sizeof(QemuOptDesc)); >> + >> + dest->name = "append_opts_list"; >> + dest->implied_opt_name = NULL; > > Not copied from an argument. Tolerable, because all you lose is the > convenience to omit name= in one place, but worth mentioning in the > function comment. > >> + dest->merge_lists = false; > > Not copied from an argument. I'm afraid the result will make no sense > if either argument has it set. Consider asserting they don't, and > documenting the requirement in the function comment. > >> + QTAILQ_INIT(&dest->head); >> + while (first && (first->desc[i].name)) { >> + if (!find_desc_by_name(dest->desc, first->desc[i].name)) { >> + dest->desc[index].name = g_strdup(first->desc[i].name); >> + dest->desc[index].help = g_strdup(first->desc[i].help); >> + dest->desc[index].type = first->desc[i].type; >> + dest->desc[index].def_value_str = >> + g_strdup(first->desc[i].def_value_str); >> + ++index; > > index++ please, for consistency with the similar increment two lines > below. > >> + } >> + i++; > > Indentation's off. > >> + } >> + i = 0; >> + while (second && (second->desc[i].name)) { >> + if (!find_desc_by_name(dest->desc, second->desc[i].name)) { >> + dest->desc[index].name = g_strdup(first->desc[i].name); >> + dest->desc[index].help = g_strdup(first->desc[i].help); >> + dest->desc[index].type = second->desc[i].type; >> + dest->desc[index].def_value_str = >> + g_strdup(second->desc[i].def_value_str); >> + ++index; >> + } >> + i++; >> + } > > We've seen this loop before. Please avoid the code duplication, as I > asked you before. > >> + dest->desc[index].name = NULL; >> + return dest; >> +} >> + >> +void free_opts_list(QemuOptsList *list) >> +{ >> + int i = 0; >> + >> + while (list && list->desc[i].name) { >> + g_free((char *)list->desc[i].name); >> + g_free((char *)list->desc[i].help); >> + g_free((char *)list->desc[i].def_value_str); >> + i++; >> + } >> + >> + g_free(list); >> +} > > Unlike most functions dealing with QemuOptsLists, this one can take null > arguments. Makes sense, but it's worth mentioning in a function > comment. > >> + >> +void print_opts_list(QemuOptsList *list) >> +{ >> + int i = 0; >> + printf("Supported options:\n"); >> + while (list && list->desc[i].name) { > > >> + printf("%-16s %s\n", list->desc[i].name, >> + list->desc[i].help ? >> + list->desc[i].help : "No description available"); > > Clearer: > > list->desc[i].help ?: "No description available"); It's a GNU extension with very little chance of becoming standard. I don't think it should be used. In this case, if you want to avoid typing (and readers reading, possibly wondering if they are identical in all cases) list->desc[i] several times, introduce a pointer variable as a shortcut. > >> + i++; >> + } >> +} > > Unlike most functions dealing with QemuOptsLists, this one can take null > arguments. Later patches feed it the result of append_opts_list(), > which can be null, but that makes little sense to me. >
于 2013-1-25 2:59, Markus Armbruster 写道: > Dong Xu Wang <wdongxu@vnet.linux.ibm.com> writes: > >> This patch will create 4 functions, count_opts_list, append_opts_list, >> free_opts_list and print_opts_list, they will used in following commits. >> >> Signed-off-by: Dong Xu Wang <wdongxu@vnet.linux.ibm.com> >> --- >> v6->v7): >> 1) Fix typo. >> >> v5->v6): >> 1) allocate enough space in append_opts_list function. >> >> include/qemu/option.h | 4 +++ >> util/qemu-option.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 94 insertions(+) >> >> diff --git a/include/qemu/option.h b/include/qemu/option.h >> index 394170a..f784c2e 100644 >> --- a/include/qemu/option.h >> +++ b/include/qemu/option.h >> @@ -156,4 +156,8 @@ int qemu_opts_print(QemuOpts *opts, void *dummy); >> int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void *opaque, >> int abort_on_failure); >> >> +QemuOptsList *append_opts_list(QemuOptsList *dest, >> + QemuOptsList *list); >> +void free_opts_list(QemuOptsList *list); >> +void print_opts_list(QemuOptsList *list); > > Please stick to the existing naming convention: qemu_opts_append(), > qemu_opts_free(), qemu_opts_print(). > Okay, will fix. >> #endif >> diff --git a/util/qemu-option.c b/util/qemu-option.c >> index 1aed418..f4bbbf8 100644 >> --- a/util/qemu-option.c >> +++ b/util/qemu-option.c >> @@ -1152,3 +1152,93 @@ int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void *opaque, >> loc_pop(&loc); >> return rc; >> } >> + >> +static size_t count_opts_list(QemuOptsList *list) >> +{ >> + size_t i = 0; >> + >> + while (list && list->desc[i].name) { >> + i++; >> + } > > Please don't invent your own way to interate over list->desc[]! Imitate > the existing code. > > for (i = 0; list && list->desc[i].name; i++) ; > > Same several times below. > Okay, will fix. >> + >> + return i; >> +} >> + >> +/* Create a new QemuOptsList and make its desc to the merge of first and second. >> + * It will allocate space for one new QemuOptsList plus enough space for >> + * QemuOptDesc in first and second QemuOptsList. First argument's QemuOptDesc >> + * members take precedence over second's. >> + */ > > Unlike most functions dealing with QemuOptsLists, this one can take null > arguments. Worth mentioning. > Okay. > Please wrap your lines a bit earlier. Column 70 is a good limit. > Okay, will use 70 column next version. >> +QemuOptsList *append_opts_list(QemuOptsList *first, >> + QemuOptsList *second) >> +{ >> + size_t num_first_options, num_second_options; >> + QemuOptsList *dest = NULL; >> + int i = 0; >> + int index = 0; >> + >> + num_first_options = count_opts_list(first); >> + num_second_options = count_opts_list(second); >> + if (num_first_options + num_second_options == 0) { >> + return NULL; >> + } > > Why do you need this extra case? Shouldn't the code below just work? > Yes, without this the code below can work. >> + >> + dest = g_malloc0(sizeof(QemuOptsList) >> + + (num_first_options + num_second_options + 1) * sizeof(QemuOptDesc)); >> + >> + dest->name = "append_opts_list"; >> + dest->implied_opt_name = NULL; > > Not copied from an argument. Tolerable, because all you lose is the > convenience to omit name= in one place, but worth mentioning in the > function comment. > Okay. >> + dest->merge_lists = false; > > Not copied from an argument. I'm afraid the result will make no sense > if either argument has it set. Consider asserting they don't, and > documenting the requirement in the function comment. Okay. > >> + QTAILQ_INIT(&dest->head); >> + while (first && (first->desc[i].name)) { >> + if (!find_desc_by_name(dest->desc, first->desc[i].name)) { >> + dest->desc[index].name = g_strdup(first->desc[i].name); >> + dest->desc[index].help = g_strdup(first->desc[i].help); >> + dest->desc[index].type = first->desc[i].type; >> + dest->desc[index].def_value_str = >> + g_strdup(first->desc[i].def_value_str); >> + ++index; > > index++ please, for consistency with the similar increment two lines > below. > Okay. >> + } >> + i++; > > Indentation's off. > Yes, will fix. >> + } >> + i = 0; >> + while (second && (second->desc[i].name)) { >> + if (!find_desc_by_name(dest->desc, second->desc[i].name)) { >> + dest->desc[index].name = g_strdup(first->desc[i].name); >> + dest->desc[index].help = g_strdup(first->desc[i].help); >> + dest->desc[index].type = second->desc[i].type; >> + dest->desc[index].def_value_str = >> + g_strdup(second->desc[i].def_value_str); >> + ++index; >> + } >> + i++; >> + } > > We've seen this loop before. Please avoid the code duplication, as I > asked you before. > Okay. >> + dest->desc[index].name = NULL; >> + return dest; >> +} >> + >> +void free_opts_list(QemuOptsList *list) >> +{ >> + int i = 0; >> + >> + while (list && list->desc[i].name) { >> + g_free((char *)list->desc[i].name); >> + g_free((char *)list->desc[i].help); >> + g_free((char *)list->desc[i].def_value_str); >> + i++; >> + } >> + >> + g_free(list); >> +} > > Unlike most functions dealing with QemuOptsLists, this one can take null > arguments. Makes sense, but it's worth mentioning in a function > comment. > Okay. >> + >> +void print_opts_list(QemuOptsList *list) >> +{ >> + int i = 0; >> + printf("Supported options:\n"); >> + while (list && list->desc[i].name) { > > >> + printf("%-16s %s\n", list->desc[i].name, >> + list->desc[i].help ? >> + list->desc[i].help : "No description available"); > > Clearer: > > list->desc[i].help ?: "No description available"); > >> + i++; >> + } >> +} > > Unlike most functions dealing with QemuOptsLists, this one can take null > arguments. Later patches feed it the result of append_opts_list(), > which can be null, but that makes little sense to me. > >
于 2013-1-26 2:13, Blue Swirl 写道: > On Thu, Jan 24, 2013 at 6:59 PM, Markus Armbruster <armbru@redhat.com> wrote: >> Dong Xu Wang <wdongxu@vnet.linux.ibm.com> writes: >> >>> This patch will create 4 functions, count_opts_list, append_opts_list, >>> free_opts_list and print_opts_list, they will used in following commits. >>> >>> Signed-off-by: Dong Xu Wang <wdongxu@vnet.linux.ibm.com> >>> --- >>> v6->v7): >>> 1) Fix typo. >>> >>> v5->v6): >>> 1) allocate enough space in append_opts_list function. >>> >>> include/qemu/option.h | 4 +++ >>> util/qemu-option.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++ >>> 2 files changed, 94 insertions(+) >>> >>> diff --git a/include/qemu/option.h b/include/qemu/option.h >>> index 394170a..f784c2e 100644 >>> --- a/include/qemu/option.h >>> +++ b/include/qemu/option.h >>> @@ -156,4 +156,8 @@ int qemu_opts_print(QemuOpts *opts, void *dummy); >>> int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void *opaque, >>> int abort_on_failure); >>> >>> +QemuOptsList *append_opts_list(QemuOptsList *dest, >>> + QemuOptsList *list); >>> +void free_opts_list(QemuOptsList *list); >>> +void print_opts_list(QemuOptsList *list); >> >> Please stick to the existing naming convention: qemu_opts_append(), >> qemu_opts_free(), qemu_opts_print(). >> >>> #endif >>> diff --git a/util/qemu-option.c b/util/qemu-option.c >>> index 1aed418..f4bbbf8 100644 >>> --- a/util/qemu-option.c >>> +++ b/util/qemu-option.c >>> @@ -1152,3 +1152,93 @@ int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void *opaque, >>> loc_pop(&loc); >>> return rc; >>> } >>> + >>> +static size_t count_opts_list(QemuOptsList *list) >>> +{ >>> + size_t i = 0; >>> + >>> + while (list && list->desc[i].name) { >>> + i++; >>> + } >> >> Please don't invent your own way to interate over list->desc[]! Imitate >> the existing code. >> >> for (i = 0; list && list->desc[i].name; i++) ; >> >> Same several times below. >> >>> + >>> + return i; >>> +} >>> + >>> +/* Create a new QemuOptsList and make its desc to the merge of first and second. >>> + * It will allocate space for one new QemuOptsList plus enough space for >>> + * QemuOptDesc in first and second QemuOptsList. First argument's QemuOptDesc >>> + * members take precedence over second's. >>> + */ >> >> Unlike most functions dealing with QemuOptsLists, this one can take null >> arguments. Worth mentioning. >> >> Please wrap your lines a bit earlier. Column 70 is a good limit. >> >>> +QemuOptsList *append_opts_list(QemuOptsList *first, >>> + QemuOptsList *second) >>> +{ >>> + size_t num_first_options, num_second_options; >>> + QemuOptsList *dest = NULL; >>> + int i = 0; >>> + int index = 0; >>> + >>> + num_first_options = count_opts_list(first); >>> + num_second_options = count_opts_list(second); >>> + if (num_first_options + num_second_options == 0) { >>> + return NULL; >>> + } >> >> Why do you need this extra case? Shouldn't the code below just work? >> >>> + >>> + dest = g_malloc0(sizeof(QemuOptsList) >>> + + (num_first_options + num_second_options + 1) * sizeof(QemuOptDesc)); >>> + >>> + dest->name = "append_opts_list"; >>> + dest->implied_opt_name = NULL; >> >> Not copied from an argument. Tolerable, because all you lose is the >> convenience to omit name= in one place, but worth mentioning in the >> function comment. >> >>> + dest->merge_lists = false; >> >> Not copied from an argument. I'm afraid the result will make no sense >> if either argument has it set. Consider asserting they don't, and >> documenting the requirement in the function comment. >> >>> + QTAILQ_INIT(&dest->head); >>> + while (first && (first->desc[i].name)) { >>> + if (!find_desc_by_name(dest->desc, first->desc[i].name)) { >>> + dest->desc[index].name = g_strdup(first->desc[i].name); >>> + dest->desc[index].help = g_strdup(first->desc[i].help); >>> + dest->desc[index].type = first->desc[i].type; >>> + dest->desc[index].def_value_str = >>> + g_strdup(first->desc[i].def_value_str); >>> + ++index; >> >> index++ please, for consistency with the similar increment two lines >> below. >> >>> + } >>> + i++; >> >> Indentation's off. >> >>> + } >>> + i = 0; >>> + while (second && (second->desc[i].name)) { >>> + if (!find_desc_by_name(dest->desc, second->desc[i].name)) { >>> + dest->desc[index].name = g_strdup(first->desc[i].name); >>> + dest->desc[index].help = g_strdup(first->desc[i].help); >>> + dest->desc[index].type = second->desc[i].type; >>> + dest->desc[index].def_value_str = >>> + g_strdup(second->desc[i].def_value_str); >>> + ++index; >>> + } >>> + i++; >>> + } >> >> We've seen this loop before. Please avoid the code duplication, as I >> asked you before. >> >>> + dest->desc[index].name = NULL; >>> + return dest; >>> +} >>> + >>> +void free_opts_list(QemuOptsList *list) >>> +{ >>> + int i = 0; >>> + >>> + while (list && list->desc[i].name) { >>> + g_free((char *)list->desc[i].name); >>> + g_free((char *)list->desc[i].help); >>> + g_free((char *)list->desc[i].def_value_str); >>> + i++; >>> + } >>> + >>> + g_free(list); >>> +} >> >> Unlike most functions dealing with QemuOptsLists, this one can take null >> arguments. Makes sense, but it's worth mentioning in a function >> comment. >> >>> + >>> +void print_opts_list(QemuOptsList *list) >>> +{ >>> + int i = 0; >>> + printf("Supported options:\n"); >>> + while (list && list->desc[i].name) { >> >> >>> + printf("%-16s %s\n", list->desc[i].name, >>> + list->desc[i].help ? >>> + list->desc[i].help : "No description available"); >> >> Clearer: >> >> list->desc[i].help ?: "No description available"); > > It's a GNU extension with very little chance of becoming standard. I > don't think it should be used. > > In this case, if you want to avoid typing (and readers reading, > possibly wondering if they are identical in all cases) list->desc[i] > several times, introduce a pointer variable as a shortcut. > >> >>> + i++; >>> + } >>> +} >> >> Unlike most functions dealing with QemuOptsLists, this one can take null >> arguments. Later patches feed it the result of append_opts_list(), >> which can be null, but that makes little sense to me. >> > > Hi, I am working with V12, but did not test completely, I will take Chinese Spring Festival vocation next 2 weeks, I will continue once I am back. :)
diff --git a/include/qemu/option.h b/include/qemu/option.h index 394170a..f784c2e 100644 --- a/include/qemu/option.h +++ b/include/qemu/option.h @@ -156,4 +156,8 @@ int qemu_opts_print(QemuOpts *opts, void *dummy); int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void *opaque, int abort_on_failure); +QemuOptsList *append_opts_list(QemuOptsList *dest, + QemuOptsList *list); +void free_opts_list(QemuOptsList *list); +void print_opts_list(QemuOptsList *list); #endif diff --git a/util/qemu-option.c b/util/qemu-option.c index 1aed418..f4bbbf8 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -1152,3 +1152,93 @@ int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void *opaque, loc_pop(&loc); return rc; } + +static size_t count_opts_list(QemuOptsList *list) +{ + size_t i = 0; + + while (list && list->desc[i].name) { + i++; + } + + return i; +} + +/* Create a new QemuOptsList and make its desc to the merge of first and second. + * It will allocate space for one new QemuOptsList plus enough space for + * QemuOptDesc in first and second QemuOptsList. First argument's QemuOptDesc + * members take precedence over second's. + */ +QemuOptsList *append_opts_list(QemuOptsList *first, + QemuOptsList *second) +{ + size_t num_first_options, num_second_options; + QemuOptsList *dest = NULL; + int i = 0; + int index = 0; + + num_first_options = count_opts_list(first); + num_second_options = count_opts_list(second); + if (num_first_options + num_second_options == 0) { + return NULL; + } + + dest = g_malloc0(sizeof(QemuOptsList) + + (num_first_options + num_second_options + 1) * sizeof(QemuOptDesc)); + + dest->name = "append_opts_list"; + dest->implied_opt_name = NULL; + dest->merge_lists = false; + QTAILQ_INIT(&dest->head); + while (first && (first->desc[i].name)) { + if (!find_desc_by_name(dest->desc, first->desc[i].name)) { + dest->desc[index].name = g_strdup(first->desc[i].name); + dest->desc[index].help = g_strdup(first->desc[i].help); + dest->desc[index].type = first->desc[i].type; + dest->desc[index].def_value_str = + g_strdup(first->desc[i].def_value_str); + ++index; + } + i++; + } + i = 0; + while (second && (second->desc[i].name)) { + if (!find_desc_by_name(dest->desc, second->desc[i].name)) { + dest->desc[index].name = g_strdup(first->desc[i].name); + dest->desc[index].help = g_strdup(first->desc[i].help); + dest->desc[index].type = second->desc[i].type; + dest->desc[index].def_value_str = + g_strdup(second->desc[i].def_value_str); + ++index; + } + i++; + } + dest->desc[index].name = NULL; + return dest; +} + +void free_opts_list(QemuOptsList *list) +{ + int i = 0; + + while (list && list->desc[i].name) { + g_free((char *)list->desc[i].name); + g_free((char *)list->desc[i].help); + g_free((char *)list->desc[i].def_value_str); + i++; + } + + g_free(list); +} + +void print_opts_list(QemuOptsList *list) +{ + int i = 0; + printf("Supported options:\n"); + while (list && list->desc[i].name) { + printf("%-16s %s\n", list->desc[i].name, + list->desc[i].help ? + list->desc[i].help : "No description available"); + i++; + } +}
This patch will create 4 functions, count_opts_list, append_opts_list, free_opts_list and print_opts_list, they will used in following commits. Signed-off-by: Dong Xu Wang <wdongxu@vnet.linux.ibm.com> --- v6->v7): 1) Fix typo. v5->v6): 1) allocate enough space in append_opts_list function. include/qemu/option.h | 4 +++ util/qemu-option.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+)