Re: [libvirt] [PATCH 03/12] tests: cleanup qemuxml2argvtest

On Tue, Mar 15, 2016 at 14:15:59 +0100, Pavel Hrdina wrote: > This removes the testFailed magic and makes the code more readable. > > Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; > --- > tests/qemuxml2argvtest.c | 62 ++++++++++++++++++++++-------------------------- > 1 file changed, 28 insertions(+), 34 deletions(-) > > diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c > index e15da37..150a50d 100644 > --- a/tests/qemuxml2argvtest.c > +++ b/tests/qemuxml2argvtest.c ... > @@ -391,15 +372,28 @@ static int testCompareXMLToArgvFiles(const char *xml, > if (virtTestCompareToFile(actualargv, cmdline) < 0) > goto out; > > + ret = 0; > + > ok: > - if (!virtTestOOMActive() && > - (flags & FLAG_EXPECT_ERROR)) { > - /* need to suppress the errors */ > + if (ret == 0 && > + ((flags & FLAG_EXPECT_ERROR) | > + (flags & FLAG_EXPECT_FAILURE))) { Are you trying to optimize the code here or is it just a typo (| vs ||)? :-) In any case if (ret == 0 && (flags & FLAG_EXPECT_ERROR || flags & FLAG_EXPECT_FAILURE)) {

or (if you prefer bit operations, which is uglier IMHO) if (ret == 0 && flags & (FLAG_EXPECT_ERROR | FLAG_EXPECT_FAILURE)) { would look a bit better. > + ret = -1; > + VIR_TEST_DEBUG("Error expected but there wasn't any.\n"); > + goto out; > + } > + if (!virtTestOOMActive()) { > + if (flags & FLAG_EXPECT_ERROR) { > + if ((log = virtTestLogContentAndReset())) > + VIR_TEST_DEBUG("Got expected error: \n%s", log); > + } else if (flags & FLAG_EXPECT_FAILURE) { > + VIR_TEST_DEBUG("Got expected failure: %s\n", > + virGetLastErrorMessage()); Indentation is off a bit. > + } > virResetLastError(); > + ret = 0; > } > > - ret = 0; > - > out: > VIR_FREE(log); > VIR_FREE(actualargv); ACK once the nits are fixed. Jirka