On Mon, Jul 11 2022, René Scharfe wrote:
> Am 10.07.22 um 16:45 schrieb Ævar Arnfjörð Bjarmason:
>>
>> On Sun, Jul 10 2022, René Scharfe wrote:
>>
>>> Some of the rules for using COPY_ARRAY instead of memcpy with sizeof are
>>> intended to reduce the number of sizeof variants to deal with. They can
>>> have unintended side effects if only they match, but not the one for the
>>> COPY_ARRAY conversion at the end.
>>
>> Since ab/cocci-unused is marked for "next" it would be really nice to
>> have this based on top so we can add tests for these transformations
>> (the topic adds a way to do that).
>
> Testing semantic patches sounds like a good idea. We can add tests later,
> once that feature landed.
Okey, I might look at this then...
>>
>> But if you don't feel like doing that this is fine too.
>>
>>> 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;
>>> -@@
>>> - memcpy(dst, src, n * sizeof(
>>> -- E[...]
>>> -+ *(E)
>>> - ))
>>> -
>>> -@@
>>> -type T;
>>> -T *ptr;
>>> -T[] arr;
>>> -expression E, 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)
>>> - )
>>> -)
>>> -
>>> @@
>>> type T;
>>> T *dst_ptr;
>>> T *src_ptr;
>>> -T[] dst_arr;
>>> -T[] src_arr;
>>> expression n;
>>> @@
>>> -(
>>> -- memcpy(dst_ptr, src_ptr, (n) * sizeof(T))
>>> +- 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)
>>> -|
>>> -- memcpy(dst_ptr, src_arr, (n) * sizeof(T))
>>> +
>>> +@@
>>> +type T;
>>> +T *dst_ptr;
>>> +T[] src_arr;
>>> +expression n;
>>> +@@
>>> +- 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)
>>> -|
>>> -- memcpy(dst_arr, src_ptr, (n) * sizeof(T))
>>> +
>>> +@@
>>> +type T;
>>> +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)
>>> -|
>>> -- memcpy(dst_arr, src_arr, (n) * sizeof(T))
>>> +
>>> +@@
>>> +type T;
>>> +T[] dst_arr;
>>> +T[] src_arr;
>>> +expression n;
>>> +@@
>>> +- 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;
>>
>> Hrm, this seems like a lot of repetition, it's here in the rules you're
>> editing already, but these repeated "sizeof" make it a lot more verbose.
>>
>> Isn't there a way to avoid this by simply wrapping this across lines, I
>> didn't test, but I think you can do this sort of thing in the cocci
>> grammar:
>>
>> - memcpy(
>> - COPY_ARRAY(
>> (
>> dst_arr
>> |
>> dst_ptr
>> )
>> ,
>> (
>> src_arr
>> |
>> src_ptr
>> )
>> ,
>> (n) *
>> - [your big sizeof alternate here]
>> )
>
> Hmm, that would match many more combinations, e.g. this one:
>
> void f(int *a, int *b, long n, int c[1]) {
> memcpy(a, b, n * sizeof(*c));
> }
>
> The elements of a, b and c have the same type, so replacing c with a
> (which a conversion to COPY_ARRAY would do) would produce the same
> object code. I can't come up with a plausible scenario like above and
> where a type change of c down the line would cause problems, but I
> also can't convince myself that no such thing can exist. Tricky.
>
> And I can't get it to format the whitespace around the third argument
> of COPY_ARRAY nicely in all cases.
>
> And it takes 37% longer on my machine.
>
> But it sure is more compact. :)
>
> @@
> type T;
> T *dst_ptr;
> T *src_ptr;
> T[] dst_arr;
> T[] src_arr;
> expression n;
> @@
> - memcpy
> + COPY_ARRAY
> (
> \( dst_ptr \| dst_arr \) ,
> \( src_ptr \| src_arr \) ,
> - (n) * \( sizeof(T)
> - \| sizeof(*(dst_ptr))
> - \| sizeof(*(dst_arr))
> - \| sizeof(*(src_ptr))
> - \| sizeof(*(src_arr))
> - \| sizeof(dst_ptr[...])
> - \| sizeof(dst_arr[...])
> - \| sizeof(src_ptr[...])
> - \| sizeof(src_arr[...])
> - \)
> + n
> )
..once those land, because cocci debug output is a lot more useful with
target test data :)
