[15/21] tests: qemuxml2argv: remove full testInfo initialization

Message ID 26cc33145b6f80854e070505d048341219d05a8b.1552574299.git.crobinso@redhat.com
State Accepted
Commit 504492a63dad8b8d01cc6f240ac224d98e7cad7e
Headers show
Series
  • tests: qemuxml2argv: support optional arguments
Related show

Commit Message

Cole Robinson March 14, 2019, 2:44 p.m.
Only initialize the fields that are passed in

Signed-off-by: Cole Robinson <crobinso@redhat.com>

---
 tests/qemuxml2argvtest.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Comments

Andrea Bolognani March 19, 2019, 2:12 p.m. | #1
On Thu, 2019-03-14 at 10:44 -0400, Cole Robinson wrote:
[...]
> +# define DO_TEST_CAPS_INTERNAL(_name, _suffix, \

>                                 arch, capsfile, stripmachinealiases, ...) \

>      do { \

>          static struct testInfo info = { \

> -            name, "." suffix, NULL, NULL, -1, 0, 0, \

> +            .name = _name, \

> +            .suffix = "." _suffix, \

>          }; \


It would probably have been a lot better to perform this kind of
conversion early in the series, and then gradually get rid of the
initialization for single members at the same time as support for
the corresponding ARG_* was implemented... But I'm not gonna ask
you to go back and do that :)

Note that we're also effectively changing the default value for
migrateFd from -1 to 0, but doing so doesn't cause any failures
since the value is only used when migrateFrom is "stdio", and in
all test cases where that is true we also already set migrateFd
explicitly.

Reviewed-by: Andrea Bolognani <abologna@redhat.com>


-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Cole Robinson March 21, 2019, 4:53 p.m. | #2
On 3/19/19 10:12 AM, Andrea Bolognani wrote:
> On Thu, 2019-03-14 at 10:44 -0400, Cole Robinson wrote:

> [...]

>> +# define DO_TEST_CAPS_INTERNAL(_name, _suffix, \

>>                                 arch, capsfile, stripmachinealiases, ...) \

>>      do { \

>>          static struct testInfo info = { \

>> -            name, "." suffix, NULL, NULL, -1, 0, 0, \

>> +            .name = _name, \

>> +            .suffix = "." _suffix, \

>>          }; \

> 

> It would probably have been a lot better to perform this kind of

> conversion early in the series, and then gradually get rid of the

> initialization for single members at the same time as support for

> the corresponding ARG_* was implemented... But I'm not gonna ask

> you to go back and do that :)

> 


Thank you :)

> Note that we're also effectively changing the default value for

> migrateFd from -1 to 0, but doing so doesn't cause any failures

> since the value is only used when migrateFrom is "stdio", and in

> all test cases where that is true we also already set migrateFd

> explicitly.

> 


Hmm I didn't notice that... but since like you say it doesn't cause test
failures, I'll leave it as is

Thanks for the reviews. I fixed all the other issues in patches 1-19 and
pushed. I'll send a v2 with patch #20+ fixed

Thanks,
Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patch

diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 176e4eff3e..b1c18c6c09 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -786,11 +786,12 @@  mymain(void)
  * the test cases should be forked using DO_TEST_CAPS_VER with the appropriate
  * version.
  */
-# define DO_TEST_CAPS_INTERNAL(name, suffix, \
+# define DO_TEST_CAPS_INTERNAL(_name, _suffix, \
                                arch, capsfile, stripmachinealiases, ...) \
     do { \
         static struct testInfo info = { \
-            name, "." suffix, NULL, NULL, -1, 0, 0, \
+            .name = _name, \
+            .suffix = "." _suffix, \
         }; \
         if (!(info.qemuCaps = qemuTestParseCapabilitiesArch(virArchFromString(arch), \
                                                             capsfile))) \
@@ -800,7 +801,7 @@  mymain(void)
         if (testInfoSetArgs(&info, __VA_ARGS__, ARG_END) < 0) \
             return EXIT_FAILURE; \
         info.flags |= FLAG_REAL_CAPS; \
-        if (virTestRun("QEMU XML-2-ARGV " name "." suffix, \
+        if (virTestRun("QEMU XML-2-ARGV " _name "." _suffix, \
                        testCompareXMLToArgv, &info) < 0) \
             ret = -1; \
         virObjectUnref(info.qemuCaps); \
@@ -841,16 +842,16 @@  mymain(void)
 
 /* All the following macros require an explicit QEMU_CAPS_* list
  * at the end of the argument list, or the NONE placeholder */
-# define DO_TEST_FULL(name, ...) \
+# define DO_TEST_FULL(_name, ...) \
     do { \
         static struct testInfo info = { \
-            name, NULL, NULL, NULL, -1, 0, 0, \
+            .name = _name, \
         }; \
         if (!(info.qemuCaps = virQEMUCapsNew())) \
             return EXIT_FAILURE; \
         if (testInfoSetArgs(&info, __VA_ARGS__, QEMU_CAPS_LAST, ARG_END) < 0) \
             return EXIT_FAILURE; \
-        if (virTestRun("QEMU XML-2-ARGV " name, \
+        if (virTestRun("QEMU XML-2-ARGV " _name, \
                        testCompareXMLToArgv, &info) < 0) \
             ret = -1; \
         virObjectUnref(info.qemuCaps); \