Re: [PATCH 1/6] scripts/coccinelle: add error-use-after-free.cocci

[Markus Armbruster]

Peter Maydell <address@hidden> writes:

> On Wed, 1 Apr 2020 at 15:44, Markus Armbruster <address@hidden> wrote:
>> Peter Maydell <address@hidden> writes:
>> > On Wed, 1 Apr 2020 at 06:07, Markus Armbruster <address@hidden> wrote:
>> > But then as a coccinelle script author I need to know which of
>> > the options I needed are standard, which are for-this-script-only,
>> > and which are just 'workflow'.
>>
>> If you're capable of writing a Coccinelle script that actually does what
>> you want, I bet you dollars to donuts that you can decide which options
>> actually affect the patch in comparably no time whatsoever ;)
>
> I use this thing maybe once a month at most, more likely once
> every three months, and the documentation is notoriously
> impenetrable. I really really don't want to have to start looking in it
> and guessing about how the original author ran the script, when
> they could have just told me.

I'm afraid we're talking part each other.

>> If you prefer to bother your reader with your personal choices, that's
>> between you and your reviewers.  Myself, I prefer less noise around the
>> signal.
>
>> If you got Coccinelle installed and know the very basics, then the
>> incantation in the script should suffice to use the script, and the
>> incantation in the commit message should suffice to reproduce the patch.
>
> So I need to now look in the git log for the script to find the commit
> message? Why not just put the command in the file and save steps?

I'm not opposed to usage comments in .cocci.

I *am* apposed to noise in usage comments.

>> Example:
>>
>>     commit 4e20c1becba3fd2e8e71a2663cefb9627fd2a6e0
>>     Author: Markus Armbruster <address@hidden>
>>     Date:   Thu Dec 13 18:51:54 2018 +0100
>>
>>         block: Replace qdict_put() by qdict_put_obj() where appropriate
>>
>>         Patch created mechanically by rerunning:
>>
>>           $  spatch --sp-file scripts/coccinelle/qobject.cocci \
>>                     --macro-file scripts/cocci-macro-file.h \
>>                     --dir block --in-place
>
> Yep, that command line would be great to see in the script file.

Except for the --dir block part, which is even worse than noise: it
suggests this is just for block/, which is wrong.

>> scripts/coccinelle/qobject.cocci has no usage comment.  I doubt it needs
>> one, but I'd certainly tolerate something like
>
>     // Usage:
>     // spatch --sp-file scripts/coccinelle/qobject.cocci \
>     //        --macro-file scripts/cocci-macro-file.h \
>     //        FILES ...
>
> I think that should be about the minimum. I think every
> .cocci file should say how it was used or is supposed to be used.

Fine with me.

> The least-effort way for the author of the script to do that is to
> simply give the command line they used to run it.

If you're capable of writing a Coccinelle script that actually does what
you want, you're certainly capable of doing better than blindly paste
from your shell history.  Kindly drop the options that are specific to
just this particular use of the script.  Keep the ones that future users
will actually need.

>> >       That's more work for the author *and* more work for the
>> > reader than just "put the command line you used into the script
>> > as a comment" -- so who's it benefiting?
>>
>> Anyone with basic Coccinelle proficiency benefits slightly from the
>> reduction of noise.
>
> How 'basic' is basic? I think that being specific is useful for
> anybody who's at my level or lower (ie, can write a script, doesn't
> do so often enough to be able to write a script or run spatch
> without looking at documentation and cribbing from other scripts
> as examples). How many people do we have at a higher level
> than that for whom this is noise? 2? 3? And people who do
> know Coccinelle well should have no difficulty in quickly
> looking at a command line and mentally filtering out the options
> that they don't feel they need.

Two proficiencies: using a script somebody else wrote, and writing
simple scripts.  Let me try to sketch just about the most basic of basic
levels for the former.  Note that I'm making *liberal* allowance for
reluctance to learn tools[*].

Understand

* that .cocci means Coccinelle
* how to install Coccinelle
* that you need to feed the .cocci to spatch
* that this produces a patch
* how to apply the patch to the tree

None of this I want to explain in every .cocci script.  All of this
I want be explained in scripts/coccinelle/README.


[*] Not a trait I like to see in craftsmen.