From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40777) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VtLFk-0000wn-UF for qemu-devel@nongnu.org; Wed, 18 Dec 2013 12:49:56 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VtLFd-0008Tk-Fb for qemu-devel@nongnu.org; Wed, 18 Dec 2013 12:49:48 -0500 Received: from cantor2.suse.de ([195.135.220.15]:43605 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VtLFd-0008TY-94 for qemu-devel@nongnu.org; Wed, 18 Dec 2013 12:49:41 -0500 Message-ID: <52B1E02D.6010008@suse.de> Date: Wed, 18 Dec 2013 18:49:33 +0100 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1387384489-25671-1-git-send-email-stefanha@redhat.com> <87y53ixew2.fsf@blackfin.pond.sub.org> In-Reply-To: <87y53ixew2.fsf@blackfin.pond.sub.org> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] qdev-monitor-test: simplify using g_assert_cmpstr() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster , Stefan Hajnoczi Cc: Paolo Bonzini , qemu-devel@nongnu.org Am 18.12.2013 18:11, schrieb Markus Armbruster: > Stefan Hajnoczi writes: >=20 >> Use g_assert_cmpstr() instead of combining g_assert() and strcmp(3). >> This simplifies the code since we no longer have to play games to >> distinguish NULL from "" using "(null)". >> >> gcc extension haters will also be happy that ?: was dropped. >> >> Suggested-by: Markus Armbruster >> Signed-off-by: Stefan Hajnoczi >> --- >> tests/qdev-monitor-test.c | 10 +++++----- >> 1 file changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/tests/qdev-monitor-test.c b/tests/qdev-monitor-test.c >> index ba7f9cc..eefaab8 100644 >> --- a/tests/qdev-monitor-test.c >> +++ b/tests/qdev-monitor-test.c >> @@ -32,8 +32,9 @@ static void test_device_add(void) >> "}}"); >> g_assert(response); >> error =3D qdict_get_qdict(response, "error"); >> - g_assert(!strcmp(qdict_get_try_str(error, "desc") ?: "", >> - "Device needs media, but drive is empty")); >> + g_assert_cmpstr(qdict_get_try_str(error, "desc"), >> + =3D=3D, >> + "Device needs media, but drive is empty"); >> QDECREF(response); >> =20 >> /* Delete the drive */ >=20 > Outside this patch's scope, but here goes anyway: why do we test the > value of member desc? Isn't that awfully fragile? >=20 > It broke once already, in Andreas's commit 75884af "virtio-blk: Convert > to QOM realize". Andreas, do you remember why you tossed the class =3D= =3D > GenericError check instead of the desc check? /me points to bonzini, who rebased it on my behalf. :) I do agree that testing error descriptions is fragile in that people may not think of running make check on a trivial textual change, e.g. typo fi= x. Andreas >=20 >> @@ -42,7 +43,7 @@ static void test_device_add(void) >> " \"command-line\": \"drive_del drive0\"" >> "}}"); >> g_assert(response); >> - g_assert(!strcmp(qdict_get_try_str(response, "return") ?: "(null)= ", "")); >> + g_assert_cmpstr(qdict_get_try_str(response, "return"), =3D=3D, ""= ); >> QDECREF(response); >> =20 >> /* Try to re-add the drive. This fails with duplicate IDs if a l= eaked >> @@ -53,8 +54,7 @@ static void test_device_add(void) >> " \"command-line\": \"drive_add pci-addr=3Dauto = if=3Dnone,id=3Ddrive0\"" >> "}}"); >> g_assert(response); >> - g_assert(!strcmp(qdict_get_try_str(response, "return") ?: "", >> - "OK\r\n")); >> + g_assert_cmpstr(qdict_get_try_str(response, "return"), =3D=3D, "O= K\r\n"); >> QDECREF(response); >> =20 >> qtest_end(); >=20 > Reviewed-by: Markus Armbruster >=20 --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg