From: Mats Kindahl <mats.kindahl@gmail.com>
To: Julia Lawall <julia.lawall@inria.fr>
Cc: cocci@inria.fr
Subject: Re: [cocci] Symbolic names in iso-files
Date: Sun, 25 Aug 2024 09:25:09 +0200 [thread overview]
Message-ID: <db729936-91ac-4851-ad65-73dc090ade6a@gmail.com> (raw)
In-Reply-To: <d6e69dbf-799f-e0a-ee55-2cac9a3555e2@inria.fr>
On 2024-08-24 20:09, Julia Lawall wrote:
>
> On Sat, 24 Aug 2024, Mats Kindahl wrote:
>
>> On 2024-08-24 14:56, Julia Lawall wrote:
>> Can you just add semicolons in your isomorphism?
>> Sent from my iPhone
>>
>>
>> Hi Julia,
>>
>> I assume you mean to write something like this:
>>
>> Statement
>> @ pg_return @
>> @@
>> PG_RETURN_VOID(); => return;
>>
>> But that fails with the same error message:
>>
>> mats@abzu:~/lang/cocci/hacks$ spatch --sp-file lwlock.cocci
>> code/lwlock.c
>> init_defs_builtins: /usr/lib/coccinelle/standard.h
>> iso main: parse error:
>> File "./postgres.iso", line 4, column 2, charpos = 29
>> around = 'PG_RETURN_VOID',
>> whole content = PG_RETURN_VOID(); => return;
> Thanks for the report. I will check on it.
>
> Even without the isomorphism, you should be able to write a disjunction in
> your semantic patch?
Yes, of course, but it becomes quite cumbersome if you have to deal with
this all the time, so this was more about an opportunity to improve the
tool by recognizing yet another common situation in which macros are
used as replacement for other constructs.
It is of course not possible to cover all weird macro uses, but the
"iterator" pattern is quite common as is this "return" pattern.
/Mats
>
> julia
>
>> Best wishes,
>> Mats Kindahl
>>
>>
>>
>> On 24 Aug 2024, at 14:23, Mats Kindahl
>> <mats.kindahl@gmail.com> wrote:
>>
>>
>> Hi all,
>>
>> In some code, there are macros used for different
>> purposes. One typical case is having a "foreach" macro,
>> but this is covered using "iterator" so that you can deal
>> with code like this:
>>
>> static Jsonb *BuildJsonObject(List *items) {
>> ListCell *cell;
>> JsonbParseState *state = NULL;
>> JsonbValue *value;
>>
>> pushJsonbValue(&state, WJB_BEGIN_OBJECT,
>> NULL);
>> foreach (cell, items) {
>> KVItem *item = (KVItem *)lfirst(cell);
>> AddJsonField(&state, item->key,
>> item->value);
>> }
>> value = pushJsonbValue(&state,
>> WJB_END_OBJECT, NULL);
>>
>> return JsonbValueToJsonb(value);
>> }
>>
>> However, if you want to make sure to deal with resource
>> allocation and make sure that you release a short-lived
>> lock, for example (grossly simplified, just to demonstrate
>> the problem):
>>
>> @@
>> expression lock, mode;
>> @@
>> LWLockAcquire(lock, mode);
>> ... when exists
>> (
>> LWLockRelease(lock);
>> |
>> + LWLockRelease(lock);
>> return;
>> )
>>
>> If fails miserably with this code:
>>
>> Datum start_worker(PG_FUNCTION_ARGS) {
>> pid_t pid;
>>
>> if (!autoprewarm)
>> ereport(ERROR,
>> (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
>> errmsg("autoprewarm is
>> disabled")));
>>
>> apw_init_shmem();
>> LWLockAcquire(&apw_state->lock,
>> LW_EXCLUSIVE);
>> pid = apw_state->bgworker_pid;
>>
>> if (pid != InvalidPid)
>> ereport(ERROR,
>> (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
>> errmsg("autoprewarm worker
>> is already running under PID %d",
>> (int)pid)));
>>
>> apw_start_leader_worker();
>>
>> PG_RETURN_VOID();
>> }
>>
>> Giving the patch, where the call to LWLockRelease should
>> be before the PG_RETURN_VOID:
>>
>> --- code/lwlock.c
>> +++ /tmp/cocci-output-12611-ff946d-lwlock.c
>> @@ -18,6 +18,7 @@ Datum
>> start_worker(PG_FUNCTION_ARGS) {
>> apw_start_leader_worker();
>>
>> PG_RETURN_VOID();
>> + LWLockRelease(&apw_state->lock);
>> }
>>
>> Since this is such a common construct, and you do not want
>> to deal with this for every potential case, this seems
>> like an obvious case for using an iso-file, so I created
>> one that looks like this, with the idea that we support
>> replacing "PG_RETURN_VOID()" with just a "return".
>>
>> Statement
>> @ pg_return @
>> @@
>> PG_RETURN_VOID() => return
>>
>> However, this fails with the following error message:
>>
>> mats@abzu:~/lang/cocci/hacks$ spatch --sp-file
>> lwlock.cocci code/lwlock.c
>> init_defs_builtins:
>> /usr/lib/coccinelle/standard.h
>> iso main: parse error:
>> File "./postgres.iso", line 4, column 19,
>> charpos = 46
>> around = '=>',
>> whole content = PG_RETURN_VOID() => return
>>
>> Which I suppose mean that there is no support for handling
>> anything but standard tokens in an .iso file.
>>
>> Have I misunderstood the situation and this is already
>> supported in some way?
>>
>> Are there any plans that add such support to the ISO
>> grammar?
>>
>> Best wishes,
>> Mats Kindahl, Timescale
>>
>>
> >
prev parent reply other threads:[~2024-08-25 7:25 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-24 12:23 [cocci] Symbolic names in iso-files Mats Kindahl
2024-08-24 12:56 ` Julia Lawall
2024-08-24 13:00 ` Mats Kindahl
2024-08-24 18:09 ` Julia Lawall
2024-08-25 7:25 ` Mats Kindahl [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=db729936-91ac-4851-ad65-73dc090ade6a@gmail.com \
--to=mats.kindahl@gmail.com \
--cc=cocci@inria.fr \
--cc=julia.lawall@inria.fr \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).