Re: coccinelle: adjustments for array.cocci?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 17.11.19 um 19:19 schrieb Markus Elfring:
>> Whitespace is not what makes the above example more complicated than the
>> equivalent rule below;
>
> A different code layout might help in a better understanding for such
> change specifications.
>
>
>> separating the pieces of simple expressions does.
>
> Will there occasionally be a need to change only the required source code parts?

Changing parts that don't need to be changed does not make sense to me.
Why do you ask and how does it relate to the example at hand?

>>>> than what we currently have:
>>>>   @@
>>>>   expression dst, src, n, E;
>>>>   @@
>>>>     memcpy(dst, src, n * sizeof(
>>>>   - E[...]
>>>>   + *(E)
>>>>     ))
>
> Are any circumstances to consider where only the essential implementation details
> should be touched by an automatic software transformation?

I don't understand this question.

>> It looks like a bug in Coccinelle to me
>
> We might stumble also on just another (temporary) software limitation.
>
>
>> and I'd like to see it fixed
>
> Would you like to support corresponding development anyhow?

I don't see me learning OCaml in the near future.  Or are you looking
for donations? :)

>> But at least until then our semantic patches need to work around it.
>
> Would another concrete fix for the currently discussed SmPL script
> be better than a “workaround”?

These are different things.  Fixes (repairs) are always welcome.  But
they should not rely on SmPL constructs that only work properly using
unreleased versions of Coccinelle.

>>> Would you like to use the SmPL code “*( \( src_ptr \| src_arr \) )” instead?
>>
>> That leaves out dst_ptr and dst_arr.
>
> How many items should finally be filtered in the discussed SmPL disjunction?

Let's see: dst and src can be pointers or array references, which makes
four combinations.  sizeof could either operate the shared type or on an
element of dst or an element of src.  An element can be accessed either
using dereference (*) or subscript ([]).  That makes five possible
variations for the sizeof, right?  So twenty combinations in total.

>> And what would it mean to match e.g. this ?
>>
>> 	memcpy(dst_ptr, src_ptr, n * sizeof(*src_arr))
>
> The Coccinelle software takes care for commutativity by isomorphisms.
> https://github.com/coccinelle/coccinelle/blob/19ee1697bf152d37a78a20cefe148775bf4b0e0d/standard.iso#L241

OK, but I had a different concern (more below).

>> At least the element size would be the same, but I'd rather shy away from
>> transforming weird cases like this automatically.
>
> Do you mean to specify additional restrictions by SmPL code?

Let's take this silly C fragment as an example:

	char *src = strdup("foo");
	size_t src_len = strlen(src);
	char *dst = malloc(src_len);
	char unrelated[17];
	memcpy(dst, src, src_len * sizeof(*unrelated));

My point is that taking the size of something that is neither source nor
destination is weird enough that it should be left alone by semantic
patches.  Matching should be precise enough to avoid false
transformations.

>>   void *memmove(void *dest, const void *src, size_t n);
>>   void *memcpy(void *dest, const void *src, size_t n);
>>
>>   COPY_ARRAY(dst, src, n)
>>   MOVE_ARRAY(dst, src, n)
>
> Can the replacement of these functions by macro calls be combined further
> by improved SmPL code?

Very likely.

>>> Possible nicer run time characteristics by the Coccinelle software.
>>
>> How much faster is it exactly?
>
> The answer will depend on efforts which you would like to invest
> in corresponding (representative) measurements.

Is that some kind of quantum effect? ;-)

When I try to convince people to apply a patch that is intended to
speed up something, I often use https://github.com/sharkdp/hyperfine
these days.

René




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux