From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp16.mail.ru (smtp16.mail.ru [94.100.176.153]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id B9A7F469719 for ; Thu, 19 Mar 2020 17:36:09 +0300 (MSK) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 13.0 \(3608.40.2.2.4\)) From: Serge Petrenko In-Reply-To: <20200316203126.GA14539@atlas> Date: Thu, 19 Mar 2020 17:36:08 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <9F39AC96-CB98-4678-8324-9369F327D4FB@tarantool.org> References: <20200305113600.65203-1-sergepetrenko@tarantool.org> <20200316203126.GA14539@atlas> Subject: Re: [Tarantool-patches] [PATCH] vclock: refactor vclock map to use type uint32_t List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Konstantin Osipov Cc: tarantool-patches@dev.tarantool.org, Vladislav Shpilevoy Hi! Thanks for the review. > 16 =D0=BC=D0=B0=D1=80=D1=82=D0=B0 2020 =D0=B3., =D0=B2 23:31, = Konstantin Osipov =D0=BD=D0=B0=D0=BF=D0=B8=D1=81= =D0=B0=D0=BB(=D0=B0): >=20 > * Serge Petrenko [20/03/05 14:36]: >> We're using an unsigned int to hold vclock map, but there is no >> guarantee that unsigned int will be 4 bytes in size to fit all the 32 >> vclock components. So use uint32_t instead. >=20 > I think there is a bunch of functions that still use unsigned to > pass the vclock value around. I=E2=80=99ve searched once again and couldn=E2=80=99t find any other = references to vclock map except the ones that were already fixed in the patch. >=20 > Besides, if you work on a fix like that, it's best to use a > typedef right away, so that it's easy to change to uint64t or 128 > bit integer in the future. Fixed. I didn=E2=80=99t touch bit_count_u32 though, it feels like a bit = of an overkill to create something like bit_count_vclock_map_t or something. diff --git a/src/box/vclock.h b/src/box/vclock.h index a8549d2a9..79e5a1bc0 100644 --- a/src/box/vclock.h +++ b/src/box/vclock.h @@ -45,6 +45,8 @@ extern "C" { #endif /* defined(__cplusplus) */ =20 +typedef uint32_t vclock_map_t; + enum { /** * The maximum number of components in vclock, should be power = of two. @@ -82,7 +84,7 @@ enum { /** Cluster vector clock */ struct vclock { /** Map of used components in lsn array */ - uint32_t map; + vclock_map_t map; /** Sum of all components of vclock. */ int64_t signature; int64_t lsn[VCLOCK_MAX]; @@ -281,7 +283,7 @@ vclock_compare_generic(const struct vclock *a, const = struct vclock *b, bool ignore_zero) { bool le =3D true, ge =3D true; - uint32_t map =3D a->map | b->map; + vclock_map_t map =3D a->map | b->map; struct bit_iterator it; bit_iterator_init(&it, &map, sizeof(map), true); =20 >> No issue, discussed in scope of >> https://github.com/tarantool/tarantool/issues/4739 >> Branch: = https://github.com/tarantool/tarantool/tree/sp/vclock-map-refactor >=20 > --=20 > Konstantin Osipov, Moscow, Russia -- Serge Petrenko sergepetrenko@tarantool.org