[Tarantool-patches] [PATCH v2 06/10] region: use aligned allocations where necessary

[Timur Safin]

: From: Vladislav Shpilevoy <v.shpilevoy at tarantool.org>
: Subject: [PATCH v2 06/10] region: use aligned allocations where necessary
: 


: diff --git a/src/box/alter.cc b/src/box/alter.cc
: index dbbbcbc44..bb4254878 100644
: --- a/src/box/alter.cc
: +++ b/src/box/alter.cc
: @@ -537,11 +537,13 @@ space_format_decode(const char *data, uint32_t
: *out_count,
:  		*fields = NULL;
:  		return 0;
:  	}
: -	size_t size = count * sizeof(struct field_def);
: +	size_t size;
:  	struct field_def *region_defs =
: -		(struct field_def *) region_alloc(region, size);
: +		region_alloc_array(region, typeof(region_defs[0]), count,
: +				   &size);

I'd say that `typeof(region_defs[0])` is very indirect way to write 
`struct field_def` :) Why you've chosen this way (and not 
simpler/clearer) way?


: @@ -2452,10 +2454,12 @@ on_replace_dd_space(struct trigger * /* trigger
: */, void *event)
:  		 * comparators must be updated.
:  		 */
:  		struct key_def **keys;
: -		size_t bsize = old_space->index_count * sizeof(keys[0]);
: -		keys = (struct key_def **) region_alloc(&fiber()->gc, bsize);
: +		size_t bsize;
: +		keys = region_alloc_array(&fiber()->gc, typeof(keys[0]),
: +					  old_space->index_count, &bsize);

Oh, now I see - for copy-paste sake :)
(I still think though the explicit `struct key_def*` would be 
clearer for the stranger passing by.)

The same question is applying multiple times below in the patch.

...
...

And joking aside, putting aside all debatable complains about 
simpler/clearer code which might be used (which you could ignore
in general), here is real complain - this is gcc preprocessor
extension, which might be not working under other C99/C11-compliant
compilers. Which we would rather avoid in a longer run (if standard 
C compatible code would not require much efforts from us, like in
these cases)

Regards,
Timur