Re: coccinelle: improve array.cocci

From: René Scharfe <l.s.r@web.de>
Date: Nov 21, 2019, 19:02 UTC
Message-ID: <4f55b06b-35f3-da06-ae86-8a4068f78027@web.de>
In-Reply-To: <d053612d-107b-fdb2-b722-6455ef068239@web.de>

Am 20.11.19 um 10:01 schrieb Markus Elfring:

Show 5 quoted lines

>> I don't like that ALLOC_ARRAY is handled in the same rule, as it is
>> quite different from the other two macros.
>
> This case distinction can share a few metavariables with the other
> transformation approach, can't it?

Can it can, but should it? In my opinion it should not; separate concerns should get their own rules. That's easier to manage for developers. I suspect it's also easier for Coccinelle to evaluate, but didn't check.

>> Coccinelle needs significantly longer to apply the new version.
>
> This can happen because of a more complete source code search pattern,
> can't it?

Perhaps.

> The data processing can benefit from parallelisation (if desired.)
> https://github.com/coccinelle/coccinelle/blob/66a1118e04a6aaf1acdae89623313c8e05158a8d/docs/manual/spatch_options.tex#L745

Right. I use MAKEFLAGS += -j6, which runs six spatch instances in parallel for the coccicheck make target of Git instead.

Show 5 quoted lines

>> Here are times for master:
>
> The SmPL script execution times can be analysed also directly with
> the help of the Coccinelle software by profiling functionality.
> https://github.com/coccinelle/coccinelle/blob/66a1118e04a6aaf1acdae89623313c8e05158a8d/docs/manual/spatch_options.tex#L736

OK, so --profile allows to analyze in which of its parts Coccinelle spends the extra time.

Show 11 quoted lines

>> The current version checks if source and destination are of the same type,
>> and whether the sizeof operand is either said type or an element of source
>> or destination.
>
> The specification of metavariables for pointer types has got some consequences.
>
>
>> The new one does not.
>
> I suggest to use a search for (pointer) expressions instead.
> This approach can trigger other consequences then.

Why don't we need to check the type?

>> So I don't see claim 4 ("Increase the precision") fulfilled,
>
> I tried to express an adjustment on the change granularity by the plus
> and minus characters at the beginning of the lines in the semantic patch.

Hmm, to me "precision" means to transform exactly those cases that are intended to be transformed, i.e. to avoid false positives and negatives. What you seem to mean here I'd rather describe as "reduce duplication".

Show 6 quoted lines

> The SmPL disjunctions provide also more common functionality now.
>
>
>> quite the opposite rather.
>
> The search for compatible pointers can become even more challenging.

It's what we currently have, in an a clunky way.

Show 6 quoted lines

>> I think an automatic transformation should only be generated if it is safe.
>
> Different expectations can occur around safety and change convenience.
>
> Would you eventually work with SmPL script variants in parallel according
> to different confidence settings?

Me? No. If I can't trust automatic transformations then I don't want them. I can already generate bugs fast enough manually, thank you very much. :)

René