Re: [Tarantool-patches] [PATCH v2 small 1/1] lsregion: introduce aligned alloc

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Hi! Thanks for the review!

On 19/05/2020 18:21, Timur Safin wrote:
> Other than small note below it's looking great, but please keep in mind...
> 
> : From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
> : Subject: [PATCH v2 small 1/1] lsregion: introduce aligned alloc
> : 
> 
> 
> : @@ -32,59 +32,62 @@
> :  #include "lsregion.h"
> : 
> :  void *
> : -lsregion_alloc_slow(struct lsregion *lsregion, size_t size, int64_t id)
> : +lsregion_aligned_reserve_slow(struct lsregion *lsregion, size_t size,
> : +			      size_t alignment, void **unaligned)
> :  {
> : -	struct lslab *slab = NULL;
> : -	size_t slab_size = lsregion->arena->slab_size;
> : +	void *pos;
> : +	struct lslab *slab;
> : +	struct slab_arena *arena = lsregion->arena;
> : +	size_t slab_size = arena->slab_size;
> : 
> :  	/* If there is an existing slab then try to use it. */
> :  	if (! rlist_empty(&lsregion->slabs.slabs)) {
> :  		slab = rlist_last_entry(&lsregion->slabs.slabs, struct lslab,
> :  					next_in_list);
> :  		assert(slab != NULL);
> : +		*unaligned = lslab_pos(slab);
> : +		pos = (void *)small_align((size_t)*unaligned, alignment);
> : +		if (pos + size <= lslab_end(slab))
> : +			return pos;
> 
> Void* pointer arithmetic is non-standard, gcc extension, and I'd 
> avoid to use it in a longer run. There are compilers is which still 
> not support it. 
> 
> Timur

Yeah, I missed this. Thanks for noticing.

====================
diff --git a/small/lsregion.c b/small/lsregion.c
index e6e347c..c01885d 100644
--- a/small/lsregion.c
+++ b/small/lsregion.c
@@ -47,7 +47,7 @@ lsregion_aligned_reserve_slow(struct lsregion *lsregion, size_t size,
 		assert(slab != NULL);
 		*unaligned = lslab_pos(slab);
 		pos = (void *)small_align((size_t)*unaligned, alignment);
-		if (pos + size <= lslab_end(slab))
+		if ((char *)pos + size <= (char *)lslab_end(slab))
 			return pos;
 	}
 	/*
@@ -88,6 +88,6 @@ lsregion_aligned_reserve_slow(struct lsregion *lsregion, size_t size,
 	}
 	*unaligned = lslab_pos(slab);
 	pos = (void *)small_align((size_t)*unaligned, alignment);
-	assert(pos + size <= lslab_end(slab));
+	assert((char *)pos + size <= (char *)lslab_end(slab));
 	return pos;
 }
diff --git a/small/lsregion.h b/small/lsregion.h
index 2fc0888..3b9a8d9 100644
--- a/small/lsregion.h
+++ b/small/lsregion.h
@@ -205,7 +205,7 @@ lsregion_aligned_reserve(struct lsregion *lsregion, size_t size,
 		assert(slab != NULL);
 		*unaligned = lslab_pos(slab);
 		void *pos = (void *)small_align((size_t)*unaligned, alignment);
-		if (pos + size <= lslab_end(slab))
+		if ((char *)pos + size <= (char *)lslab_end(slab))
 			return pos;
 	}
 	return lsregion_aligned_reserve_slow(lsregion, size, alignment,