Re: [Tarantool-patches] [PATCH] vclock: refactor vclock map to use type uint32_t

[Serge Petrenko <sergepetrenko@tarantool.org>]

Hi! Thanks for the review.


> 16 марта 2020 г., в 23:31, Konstantin Osipov <kostja.osipov@gmail.com> написал(а):
> 
> * Serge Petrenko <sergepetrenko@tarantool.org> [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.
> 
> I think there is a bunch of functions that still use unsigned to
> pass the vclock value around.

I’ve searched once again and couldn’t find any other references to vclock map
except the ones that were already fixed in the patch.

> 
> 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’t 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) */
 
+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 = true, ge = true;
-	uint32_t map = a->map | b->map;
+	vclock_map_t map = a->map | b->map;
 	struct bit_iterator it;
 	bit_iterator_init(&it, &map, sizeof(map), true);
 


>> No issue, discussed in scope of
>> https://github.com/tarantool/tarantool/issues/4739
>> Branch: https://github.com/tarantool/tarantool/tree/sp/vclock-map-refactor
> 
> -- 
> Konstantin Osipov, Moscow, Russia

--
Serge Petrenko
sergepetrenko@tarantool.org