Re: [PATCH] git-compat-util.h: introduce CALLOC(x)

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Dec 7, 2022, 03:51 UTC
Message-ID: <221207.867cz3c1cp.gmgdl@evledraar.gmail.com>
In-Reply-To: <Y4/6eDxUeSLYss/a@nand.local>

On Tue, Dec 06 2022, Taylor Blau wrote:

Show 34 quoted lines

> On Mon, Dec 05, 2022 at 08:43:50PM -0500, Jeff King wrote:
>> On Mon, Dec 05, 2022 at 05:36:25PM -0500, Taylor Blau wrote:
>>
>> > On Mon, Dec 05, 2022 at 10:01:11PM +0100, René Scharfe wrote:
>> > > This rule would turn this code:
>> > >
>> > > 	struct foo *bar = xcalloc(1, sizeof(*bar));
>> > > 	int i;
>> > >
>> > > ... into:
>> > >
>> > > 	struct foo *bar;
>> > > 	CALLOC(bar);
>> > > 	int i;
>> > >
>> > > ... which violates the coding guideline to not mix declarations and
>> > > statements (-Wdeclaration-after-statement).
>> >
>> > Yeah, I was wondering about this myself when I wrote this part of the
>> > Coccinelle patch.
>> >
>> > Is there an intelligent way to tell it to put the first statement after
>> > all declarations? I couldn't find anything after a quick scan of the
>> > documentation nor our own patches.
>>
>> It feels like generating the code as above is not the end of the world.
>> The most valuable thing that coccinelle is doing here is _finding_ the
>> location, and telling you "it's supposed to be like this". It is great
>> when the "this" post-image is perfect and doesn't need further tweaking.
>
> I have to agree. If Coccinelle can generate the right output; great. But
> if it can't, the amount of additional work to reorganize an already
> generated and mostly correct *.patch from the tool seems minimal by
> comparison.

It can, but you need to write your semantic patch to match your intent. If you write e.g.:

	- int x;
        + int y;
        + foo();

That means "add the int y and foo() line right after that "int x" line you removed.

Whereas what you want in this case is closer to:

 - match the "int x" line
 - remove or amend it
 - skip past all subsequent declarations
 - skip past all code that isn't referring to the "x variable?
 - insert the "CALLOC_ARRAY" (or whatever) before that first "x" use.

I don't know offhand how to match this, but presumably it's some mixture of the wildcard syntax ("...", "<... ...>" etc.) and matching a "statement", or maybe marking the "int x" with the "@pos" syntax, and referring to that position again.

I usually just browse through the coccinelle.git for *.cocci examples and/or read the PDF (*not* the manual page, which discusses almost none of the syntax) documentation.

See:

	git grep -F '...' -- contrib/coccinelle

For some in-tree use of this, the unused.cocci I added recently is probably the closest equivalent to what you'll want.