From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60566) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eJHrR-0004jE-2W for qemu-devel@nongnu.org; Mon, 27 Nov 2017 06:46:09 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eJHrM-00072n-VB for qemu-devel@nongnu.org; Mon, 27 Nov 2017 06:46:05 -0500 Received: from mx1.redhat.com ([209.132.183.28]:50778) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eJHrM-00071R-MV for qemu-devel@nongnu.org; Mon, 27 Nov 2017 06:46:00 -0500 Date: Mon, 27 Nov 2017 11:45:52 +0000 From: "Daniel P. Berrange" Message-ID: <20171127114552.GI32413@redhat.com> Reply-To: "Daniel P. Berrange" References: <20171124143206.28833-1-rkagan@virtuozzo.com> <64158eb0-98a0-5d06-7545-cb3a08a3c472@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <64158eb0-98a0-5d06-7545-cb3a08a3c472@redhat.com> Subject: Re: [Qemu-devel] [PATCH] util: add is_equal to UUID API List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Thomas Huth Cc: Roman Kagan , qemu-devel@nongnu.org, Fam Zheng , "Denis V. Lunev" On Fri, Nov 24, 2017 at 06:34:03PM +0100, Thomas Huth wrote: > On 24.11.2017 15:32, Roman Kagan wrote: > > It's going to be useful, in particular, in VMBus code massively using > > uuids aka GUIDs. > > > > Signed-off-by: Roman Kagan > > --- > > include/qemu/uuid.h | 2 ++ > > tests/test-uuid.c | 24 ++++++++++++++++++++++++ > > util/uuid.c | 7 ++++++- > > 3 files changed, 32 insertions(+), 1 deletion(-) > > > > diff --git a/include/qemu/uuid.h b/include/qemu/uuid.h > > index afe4840296..09489ce5c5 100644 > > --- a/include/qemu/uuid.h > > +++ b/include/qemu/uuid.h > > @@ -48,6 +48,8 @@ void qemu_uuid_generate(QemuUUID *out); > > > > int qemu_uuid_is_null(const QemuUUID *uu); > > > > +int qemu_uuid_is_equal(const QemuUUID *lhv, const QemuUUID *rhv); > > + > > void qemu_uuid_unparse(const QemuUUID *uuid, char *out); > > > > char *qemu_uuid_unparse_strdup(const QemuUUID *uuid); > > diff --git a/tests/test-uuid.c b/tests/test-uuid.c > > index d3a2791fd4..c6c8148117 100644 > > --- a/tests/test-uuid.c > > +++ b/tests/test-uuid.c > > @@ -116,6 +116,29 @@ static void test_uuid_is_null(void) > > g_assert_false(qemu_uuid_is_null(&uuid_not_null_2)); > > } > > > > +static void test_uuid_is_equal(void) > > +{ > > + int i; > > + QemuUUID uuid; > > + QemuUUID uuid_null = { }; > > + QemuUUID uuid_not_null = { { { > > + 0x58, 0x6e, 0xce, 0x27, 0x7f, 0x09, 0x41, 0xe0, > > + 0x9e, 0x74, 0xe9, 0x01, 0x31, 0x7e, 0x9d, 0x42 > > + } } }; > > + QemuUUID uuid_null_2 = uuid_null; > > + QemuUUID uuid_not_null_2 = uuid_not_null; > > + > > + g_assert(qemu_uuid_is_equal(&uuid_null, &uuid_null_2)); > > + g_assert(qemu_uuid_is_equal(&uuid_not_null, &uuid_not_null_2)); > > + g_assert_false(qemu_uuid_is_equal(&uuid_null, &uuid_not_null)); > > + > > + for (i = 0; i < 100; ++i) { > > + qemu_uuid_generate(&uuid); > > + g_assert_false(qemu_uuid_is_equal(&uuid_null, &uuid)); > > + g_assert_false(qemu_uuid_is_equal(&uuid_not_null, &uuid)); > > Isn't there a very low chance that the last line triggers by accident? > Or uuid_no_null guaranteed to not match the generated one? In the latter > case, a comment with a short explanation might be helpful here... Regardless of that question, I think this is rather overkill when we are just validating the memcmp() works correct in qemu_uuid_is_equal. The for() loop here is not really adding any value over what the earlier asserts already did. In fact the for() loop is arguably testing the qemu_uuid_generate method, so better done in a separate unit test. IOW, I would just suggest deleting this for() loop as its adding no value. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|