Re: [PATCH] cocci: extend MEMZERO_ARRAY() rules

[Junio C Hamano <gitster@pobox.com>]

Toon Claes <toon@iotcl.com> writes:

> Recently the MEMZERO_ARRAY() macro was introduced. In that commit also
> coccinelle rules were added to capture cases that can be converted to
> use that macro.
>
> Later a few more cases were manually converted to use the macro, but
> coccinelle didn't capture those. Extend the rules to capture those as
> well.
>
> In various cases the code could be further beautified by removing
> parentheses which are no longer needed. Modify the coccinelle rules to
> optimize those as well and fix them.
>
> During conversion indentation also used spaces where tabs should be
> used, fix that in one go.
>
> Signed-off-by: Toon Claes <toon@iotcl.com>
> ---
>  contrib/coccinelle/array.cocci | 36 ++++++++++++++++++++++++++++------
>  diffcore-delta.c               |  2 +-
>  ewah/bitmap.c                  |  4 ++--
>  3 files changed, 33 insertions(+), 9 deletions(-)
>
> diff --git a/contrib/coccinelle/array.cocci b/contrib/coccinelle/array.cocci
> index d306f6a21e..e71baea00b 100644
> --- a/contrib/coccinelle/array.cocci
> +++ b/contrib/coccinelle/array.cocci
> @@ -107,9 +107,32 @@ type T;
>  T *ptr;
>  expression n;
>  @@
> -- memset(ptr, \( 0x0 \| 0 \), n * \( sizeof(T)
> --                                 \| sizeof(*ptr)
> --                                 \) )
> +- memset(ptr, \( 0 \| '\0' \), \( (n) \| n \) * \( sizeof(T)
> +-                                               \| sizeof(ptr[...])
> +-                                               \| sizeof(*ptr)
> +-                                               \) )

It is kind-a surprising that (n) with and without parentheses must
be presented for the conversion to happen, while the tool clearly
understands that 0 and 0x0 are equivalent ...

> ++ MEMZERO_ARRAY(ptr, n)
> +
> +@@
> +type T;
> +T *ptr;
> +expression n;
> +@@
> +- memset(ptr, \( 0 \| '\0' \), \( sizeof(T)
> +-                              \| sizeof(ptr[...])
> +-                              \| sizeof(*ptr)
> +-                              \) * \( (n) \| n \) )

... and yet the tool does not understand the equivalence between
'\0' and 0, nor (A * B) and (B * A).  Puzzling, but OK.

And the pattern, with *ptr  substituted to ptr[], repeats.

> diff --git a/diffcore-delta.c b/diffcore-delta.c
> index 2de9e9ccff..2b7db39983 100644
> --- a/diffcore-delta.c
> +++ b/diffcore-delta.c
> @@ -135,7 +135,7 @@ static struct spanhash_top *hash_chars(struct repository *r,
>  			      st_mult(sizeof(struct spanhash), (size_t)1 << i)));
>  	hash->alloc_log2 = i;
>  	hash->free = INITIAL_FREE(i);
> -	MEMZERO_ARRAY(hash->data, ((size_t)1 << i));
> +	MEMZERO_ARRAY(hash->data, (size_t)1 << i);
>
>  	n = 0;
>  	accum1 = accum2 = 0;
> diff --git a/ewah/bitmap.c b/ewah/bitmap.c
> index bf878bf876..c378e0ab78 100644
> --- a/ewah/bitmap.c
> +++ b/ewah/bitmap.c
> @@ -46,7 +46,7 @@ static void bitmap_grow(struct bitmap *self, size_t word_alloc)
>  {
>  	size_t old_size = self->word_alloc;
>  	ALLOC_GROW(self->words, word_alloc, self->word_alloc);
> -	MEMZERO_ARRAY(self->words + old_size, (self->word_alloc - old_size));
> +	MEMZERO_ARRAY(self->words + old_size, self->word_alloc - old_size);
>  }
>
>  void bitmap_set(struct bitmap *self, size_t pos)
> @@ -192,7 +192,7 @@ void bitmap_or_ewah(struct bitmap *self, struct ewah_bitmap *other)
>  		self->word_alloc = other_final;
>  		REALLOC_ARRAY(self->words, self->word_alloc);
>  		MEMZERO_ARRAY(self->words + original_size,
> -		              (self->word_alloc - original_size));
> +			      self->word_alloc - original_size);
>  	}
>
>  	ewah_iterator_init(&it, other);

OK.  These do look easier to follow without extra parentheses.

Will queue.  Thanks.