[Qemu-devel] [PATCH V15 3/6] Create four QemuOptsList related functions
From: Dong Xu Wang wdon...@linux.vnet.ibm.com This patch will create 4 functions, count_opts_list, qemu_opts_append, qemu_opts_free and qemu_opts_print_help, they will be used in following commits. v12-v13: 1) simply assert that neither argument has merge_lists set. 2) drop superfluous paranthesesis around p == first. v11-v12: 1) renmae functions. 2) fix loop styles and code styles. 3) qemu_opts_apend will not return NULL now. 4) merge_lists value is from arguments in qemu_opts_append. v6-v7: 1) Fix typo. v5-v6: 1) allocate enough space in append_opts_list function. Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com Signed-off-by: Dongxu Wang wdon...@linux.vnet.ibm.com --- include/qemu/option.h | 3 ++ util/qemu-option.c| 82 +++ 2 files changed, 85 insertions(+) diff --git a/include/qemu/option.h b/include/qemu/option.h index b928ab0..c7a5c14 100644 --- a/include/qemu/option.h +++ b/include/qemu/option.h @@ -157,4 +157,7 @@ int qemu_opts_print(QemuOpts *opts); int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void *opaque, int abort_on_failure); +QemuOptsList *qemu_opts_append(QemuOptsList *first, QemuOptsList *second); +void qemu_opts_free(QemuOptsList *list); +void qemu_opts_print_help(QemuOptsList *list); #endif diff --git a/util/qemu-option.c b/util/qemu-option.c index bd2acdc..e3f482f 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -1218,3 +1218,85 @@ 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; + +for (i = 0; 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. + * The result's name and implied_opt_name are not copied from them. + * Both merge_lists should not be set. Both list can be NULL. + */ +QemuOptsList *qemu_opts_append(QemuOptsList *first, + QemuOptsList *second) +{ +size_t num_first_opts, num_second_opts; +QemuOptsList *dest = NULL; +int i = 0; +int index = 0; +QemuOptsList *p = first; + +num_first_opts = count_opts_list(first); +num_second_opts = count_opts_list(second); + +dest = g_malloc0(sizeof(QemuOptsList) ++ (num_first_opts + num_second_opts + 1) * sizeof(QemuOptDesc)); + +dest-name = append_opts_list; +dest-implied_opt_name = NULL; +assert((!first || !first-merge_lists) + (!second || !second-merge_lists)); +QTAILQ_INIT(dest-head); + +for (i = 0; p p-desc[i].name; i++) { +if (!find_desc_by_name(dest-desc, p-desc[i].name)) { +dest-desc[index].name = g_strdup(p-desc[i].name); +dest-desc[index].help = g_strdup(p-desc[i].help); +dest-desc[index].type = p-desc[i].type; +dest-desc[index].def_value_str = +g_strdup(p-desc[i].def_value_str); +index++; +} +if (p == first p !p-desc[i].name) { +p = second; +i = 0; +} +} +dest-desc[index].name = NULL; +return dest; +} + +/* free a QemuOptsList, can accept NULL as arguments */ +void qemu_opts_free(QemuOptsList *list) +{ +int i = 0; + +for (i = 0; list list-desc[i].name; i++) { +g_free((char *)list-desc[i].name); +g_free((char *)list-desc[i].help); +g_free((char *)list-desc[i].def_value_str); +} + +g_free(list); +} + +void qemu_opts_print_help(QemuOptsList *list) +{ +int i = 0; +printf(Supported options:\n); +for (i = 0; list list-desc[i].name; i++) { +printf(%-16s %s\n, list-desc[i].name, +list-desc[i].help ? +list-desc[i].help : No description available); +} +} -- 1.7.11.7
Re: [Qemu-devel] [PATCH V15 3/6] Create four QemuOptsList related functions
On 05/30/2013 03:55 AM, Dongxu Wang wrote: From: Dong Xu Wang wdon...@linux.vnet.ibm.com This patch will create 4 functions, count_opts_list, qemu_opts_append, s/will create/creates/ - commit messages make the most sense when written in present tense qemu_opts_free and qemu_opts_print_help, they will be used in following commits. Again, this portion... v12-v13: 1) simply assert that neither argument has merge_lists set. 2) drop superfluous paranthesesis around p == first. v11-v12: 1) renmae functions. 2) fix loop styles and code styles. 3) qemu_opts_apend will not return NULL now. 4) merge_lists value is from arguments in qemu_opts_append. v6-v7: 1) Fix typo. v5-v6: 1) allocate enough space in append_opts_list function. ...belongs after '---', and Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com Signed-off-by: Dongxu Wang wdon...@linux.vnet.ibm.com your s-o-b is unusual. + +static size_t count_opts_list(QemuOptsList *list) +{ +size_t i = 0; + +for (i = 0; list list-desc[i].name; i++) { No need to initialize i to 0 in two places. +; +} + +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. + * The result's name and implied_opt_name are not copied from them. + * Both merge_lists should not be set. Both list can be NULL. + */ +QemuOptsList *qemu_opts_append(QemuOptsList *first, + QemuOptsList *second) +{ +size_t num_first_opts, num_second_opts; +QemuOptsList *dest = NULL; +int i = 0; +int index = 0; +QemuOptsList *p = first; + +num_first_opts = count_opts_list(first); +num_second_opts = count_opts_list(second); + +dest = g_malloc0(sizeof(QemuOptsList) ++ (num_first_opts + num_second_opts + 1) * sizeof(QemuOptDesc)); + +dest-name = append_opts_list; +dest-implied_opt_name = NULL; +assert((!first || !first-merge_lists) + (!second || !second-merge_lists)); +QTAILQ_INIT(dest-head); + +for (i = 0; p p-desc[i].name; i++) { Again, a double-initialization of i [1] +if (!find_desc_by_name(dest-desc, p-desc[i].name)) { +dest-desc[index].name = g_strdup(p-desc[i].name); +dest-desc[index].help = g_strdup(p-desc[i].help); +dest-desc[index].type = p-desc[i].type; +dest-desc[index].def_value_str = +g_strdup(p-desc[i].def_value_str); Do we really have to strdup these elements, or are we guaranteed that the scope of the original first/second list is always larger than the scope of the merged list, and can thus share the existing pointer rather than creating copies? [2] +index++; +} +if (p == first p !p-desc[i].name) { +p = second; +i = 0; +} +} +dest-desc[index].name = NULL; +return dest; +} + +/* free a QemuOptsList, can accept NULL as arguments */ +void qemu_opts_free(QemuOptsList *list) +{ +int i = 0; + +for (i = 0; list list-desc[i].name; i++) { [1] and again for double initialization of i +g_free((char *)list-desc[i].name); +g_free((char *)list-desc[i].help); +g_free((char *)list-desc[i].def_value_str); [2] The fact that you have to cast away const is a sign that maybe you shouldn't be storing strdup'd data in these pointers in the first place. +} + +g_free(list); +} + +void qemu_opts_print_help(QemuOptsList *list) +{ +int i = 0; +printf(Supported options:\n); +for (i = 0; list list-desc[i].name; i++) { [1] and another +printf(%-16s %s\n, list-desc[i].name, +list-desc[i].help ? +list-desc[i].help : No description available); +} +} -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH V15 3/6] Create four QemuOptsList related functions
On 2013/5/31 5:43, Eric Blake wrote: On 05/30/2013 03:55 AM, Dongxu Wang wrote: From: Dong Xu Wang wdon...@linux.vnet.ibm.com This patch will create 4 functions, count_opts_list, qemu_opts_append, s/will create/creates/ - commit messages make the most sense when written in present tense qemu_opts_free and qemu_opts_print_help, they will be used in following commits. Again, this portion... v12-v13: 1) simply assert that neither argument has merge_lists set. 2) drop superfluous paranthesesis around p == first. v11-v12: 1) renmae functions. 2) fix loop styles and code styles. 3) qemu_opts_apend will not return NULL now. 4) merge_lists value is from arguments in qemu_opts_append. v6-v7: 1) Fix typo. v5-v6: 1) allocate enough space in append_opts_list function. ...belongs after '---', and Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com Signed-off-by: Dongxu Wang wdon...@linux.vnet.ibm.com your s-o-b is unusual. + +static size_t count_opts_list(QemuOptsList *list) +{ +size_t i = 0; + +for (i = 0; list list-desc[i].name; i++) { No need to initialize i to 0 in two places. Okay. +; +} + +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. + * The result's name and implied_opt_name are not copied from them. + * Both merge_lists should not be set. Both list can be NULL. + */ +QemuOptsList *qemu_opts_append(QemuOptsList *first, + QemuOptsList *second) +{ +size_t num_first_opts, num_second_opts; +QemuOptsList *dest = NULL; +int i = 0; +int index = 0; +QemuOptsList *p = first; + +num_first_opts = count_opts_list(first); +num_second_opts = count_opts_list(second); + +dest = g_malloc0(sizeof(QemuOptsList) ++ (num_first_opts + num_second_opts + 1) * sizeof(QemuOptDesc)); + +dest-name = append_opts_list; +dest-implied_opt_name = NULL; +assert((!first || !first-merge_lists) + (!second || !second-merge_lists)); +QTAILQ_INIT(dest-head); + +for (i = 0; p p-desc[i].name; i++) { Again, a double-initialization of i [1] Okay. +if (!find_desc_by_name(dest-desc, p-desc[i].name)) { +dest-desc[index].name = g_strdup(p-desc[i].name); +dest-desc[index].help = g_strdup(p-desc[i].help); +dest-desc[index].type = p-desc[i].type; +dest-desc[index].def_value_str = +g_strdup(p-desc[i].def_value_str); Do we really have to strdup these elements, or are we guaranteed that the scope of the original first/second list is always larger than the scope of the merged list, and can thus share the existing pointer rather than creating copies? [2] I think yes, will use pointer directly in next version. +index++; +} +if (p == first p !p-desc[i].name) { +p = second; +i = 0; +} +} +dest-desc[index].name = NULL; +return dest; +} + +/* free a QemuOptsList, can accept NULL as arguments */ +void qemu_opts_free(QemuOptsList *list) +{ +int i = 0; + +for (i = 0; list list-desc[i].name; i++) { [1] and again for double initialization of i +g_free((char *)list-desc[i].name); +g_free((char *)list-desc[i].help); +g_free((char *)list-desc[i].def_value_str); [2] The fact that you have to cast away const is a sign that maybe you shouldn't be storing strdup'd data in these pointers in the first place. +} + +g_free(list); +} + +void qemu_opts_print_help(QemuOptsList *list) +{ +int i = 0; +printf(Supported options:\n); +for (i = 0; list list-desc[i].name; i++) { [1] and another +printf(%-16s %s\n, list-desc[i].name, +list-desc[i].help ? +list-desc[i].help : No description available); +} +}