Re: [PATCH] builtin/mv.c: use correct type to compute size of an array element

["René Scharfe" <l.s.r@web.de>]

Am 07.07.22 um 21:11 schrieb René Scharfe:
> Am 07.07.22 um 20:10 schrieb Junio C Hamano:
>>     @@
>>     type T;
>>     T *dst_ptr;
>>     T *src_ptr;
>>     T[] dst_arr;
>>     T[] src_arr;
>>     expression n;
>>     @@
>>     (
>>     - memcpy(dst_ptr, src_ptr, (n) * sizeof(T))
>>     + COPY_ARRAY(dst_ptr, src_ptr, n)
>>     |
>>     - memcpy(dst_ptr, src_arr, (n) * sizeof(T))
>>     + COPY_ARRAY(dst_ptr, src_arr, n)
>>     |
>>     - memcpy(dst_arr, src_ptr, (n) * sizeof(T))
>>     + COPY_ARRAY(dst_arr, src_ptr, n)
>>     |
>>     - memcpy(dst_arr, src_arr, (n) * sizeof(T))
>>     + COPY_ARRAY(dst_arr, src_arr, n)
>>     )
>>
>> I take it that thanks to the earlier "meh -- between sizeof(*p) and
>> sizeof(p[0]) there is no reason to prefer one over the other" and
>> "oh, no, we should prefer sizeof(*p) not sizeof(typeof(*p)) but this
>> one is the other way around" rules, this one only has to deal with
>> sizeof(T).
>>
>> Am I reading it correctly?
>
> Yes.  Without the ugly normalization step in the middle could either
> use twelve cases instead of four here or use inline alternatives,
> e.g.:
>
> type T;
> T *dst_ptr;
> T *src_ptr;
> T[] dst_arr;
> T[] src_arr;
> expression n;
> @@
> (
> - memcpy(dst_ptr, src_ptr, (n) * \( sizeof(*(dst_ptr)) \| sizeof(*(src_ptr)) \| sizeof(T) \) )
> + COPY_ARRAY(dst_ptr, src_ptr, n)
> |
> - memcpy(dst_ptr, src_arr, (n) * \( sizeof(*(dst_ptr)) \| sizeof(*(src_arr)) \| sizeof(T) \) )
> + COPY_ARRAY(dst_ptr, src_arr, n)
> |
> - memcpy(dst_arr, src_ptr, (n) * \( sizeof(*(dst_arr)) \| sizeof(*(src_ptr)) \| sizeof(T) \) )
> + COPY_ARRAY(dst_arr, src_ptr, n)
> |
> - memcpy(dst_arr, src_arr, (n) * \( sizeof(*(dst_arr)) \| sizeof(*(src_arr)) \| sizeof(T) \) )
> + COPY_ARRAY(dst_arr, src_arr, n)
> )
>
> I seem to remember that rules like this missed some cases, but perhaps
> that's no longer an issue with the latest Coccinelle version?

Not a problem, it seems; at least Coccinelle 1.1.1 is still able to
recreate the conversions from 45ccef87b3 (use COPY_ARRAY, 2016-09-25)
and 921d49be86 (use COPY_ARRAY for copying arrays, 2019-06-15) with the
patch below, which removes the normalization rules.  It increases the
processing time for array.cocci from 53s to 66s for me, though.  Worth
the increased precision and clarity?

René

---
 contrib/coccinelle/array.cocci | 82 +++++++++++++++++-----------------
 1 file changed, 40 insertions(+), 42 deletions(-)

diff --git a/contrib/coccinelle/array.cocci b/contrib/coccinelle/array.cocci
index 9a4f00cb1b..aa75937950 100644
--- a/contrib/coccinelle/array.cocci
+++ b/contrib/coccinelle/array.cocci
@@ -1,60 +1,58 @@
 @@
-expression dst, src, n, E;
+type T;
+T *dst_ptr;
+T *src_ptr;
+expression n;
 @@
-  memcpy(dst, src, n * sizeof(
-- E[...]
-+ *(E)
-  ))
+- memcpy(dst_ptr, src_ptr, (n) * \( sizeof(T)
+-                                \| sizeof(*(dst_ptr))
+-                                \| sizeof(*(src_ptr))
+-                                \| sizeof(dst_ptr[...])
+-                                \| sizeof(src_ptr[...])
+-                                \) )
++ COPY_ARRAY(dst_ptr, src_ptr, n)

 @@
 type T;
-T *ptr;
-T[] arr;
-expression E, n;
+T *dst_ptr;
+T[] src_arr;
+expression n;
 @@
-(
-  memcpy(ptr, E,
-- n * sizeof(*(ptr))
-+ n * sizeof(T)
-  )
-|
-  memcpy(arr, E,
-- n * sizeof(*(arr))
-+ n * sizeof(T)
-  )
-|
-  memcpy(E, ptr,
-- n * sizeof(*(ptr))
-+ n * sizeof(T)
-  )
-|
-  memcpy(E, arr,
-- n * sizeof(*(arr))
-+ n * sizeof(T)
-  )
-)
+- memcpy(dst_ptr, src_arr, (n) * \( sizeof(T)
+-                                \| sizeof(*(dst_ptr))
+-                                \| sizeof(*(src_arr))
+-                                \| sizeof(dst_ptr[...])
+-                                \| sizeof(src_arr[...])
+-                                \) )
++ COPY_ARRAY(dst_ptr, src_arr, n)

 @@
 type T;
-T *dst_ptr;
+T[] dst_arr;
 T *src_ptr;
+expression n;
+@@
+- memcpy(dst_arr, src_ptr, (n) * \( sizeof(T)
+-                                \| sizeof(*(dst_arr))
+-                                \| sizeof(*(src_ptr))
+-                                \| sizeof(dst_arr[...])
+-                                \| sizeof(src_ptr[...])
+-                                \) )
++ COPY_ARRAY(dst_arr, src_ptr, n)
+
+@@
+type T;
 T[] dst_arr;
 T[] src_arr;
 expression n;
 @@
-(
-- memcpy(dst_ptr, src_ptr, (n) * sizeof(T))
-+ COPY_ARRAY(dst_ptr, src_ptr, n)
-|
-- memcpy(dst_ptr, src_arr, (n) * sizeof(T))
-+ COPY_ARRAY(dst_ptr, src_arr, n)
-|
-- memcpy(dst_arr, src_ptr, (n) * sizeof(T))
-+ COPY_ARRAY(dst_arr, src_ptr, n)
-|
-- memcpy(dst_arr, src_arr, (n) * sizeof(T))
+- memcpy(dst_arr, src_arr, (n) * \( sizeof(T)
+-                                \| sizeof(*(dst_arr))
+-                                \| sizeof(*(src_arr))
+-                                \| sizeof(dst_arr[...])
+-                                \| sizeof(src_arr[...])
+-                                \) )
 + COPY_ARRAY(dst_arr, src_arr, n)
-)

 @@
 type T;
--
2.37.0