[Qemu-devel] [PATCH V15 3/6] Create four QemuOptsList related functions

2013-05-30 Thread Dongxu Wang
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

2013-05-30 Thread Eric Blake
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

2013-05-30 Thread Dongxu Wang

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);
+}
+}