Re: [cocci] How to add error handling code before first use

[Mats Kindahl <mats.kindahl@gmail.com>]

On 2024-11-13 16:13, Julia Lawall wrote:
>
> On Mon, 11 Nov 2024, Julia Lawall wrote:
>
>>
>> On Mon, 11 Nov 2024, Mats Kindahl wrote:
>>
>>> Hi again,
>>>
>>> I am trying to match code like the following and add error handling to it:
>>>
>>>        #include <postgres.h>
>>>
>>>        void foo(Oid id) {
>>>          HeapTuple tuple;
>>>          Form_pg_proc fd;
>>>
>>>          tuple = SearchSysCache1(PROCOID, ObjectIdGetDatum(id));
>>>          fd = (Form_pg_proc)GETSTRUCT(tuple);
>>>
>>>          if (strcmp("magic", NameStr(fd->proname)) != 0)
>>>            printf("Bad magic\n");
>>>        }
>>>
>>> The semantic patch I have for this is:
>>>
>>>        @@
>>>        identifier tuple;
>>>        expression id, cache;
>>>        statement S;
>>>        @@
>>>          tuple = SearchSysCache1(cache, ObjectIdGetDatum(id));
>>>          ... when != tuple
>>>        (
>>>          if (<+... HeapTupleIsValid(tuple) ...+>) S
>>>        |
>>>        + if (!HeapTupleIsValid(tuple))
>>>        +   elog(ERROR, "cache lookup failed for %u", id);
>>>          (<+...tuple...+>);
>>>        )
> Hello,
>
> The problem is that first Coccinelle uses the isomorphism and drops the
> ().  Then the code that merges the + code with the context code sees code
> added before a <+... ...+>, which it doesn't like.
>
> To solve this you should be able to do something like the following (I
> simplified it a bit):
>
> @@
> identifier tuple;
> expression id, cache;
> statement S;
> @@
>     tuple = SearchSysCache1(cache, ObjectIdGetDatum(id));
>     ... when != tuple
> (
> + if (!HeapTupleIsValid(tuple))
> +     elog(ERROR, "cache lookup failed for %u", id);
>     S
> &
>     (<+...tuple...+>);
> )
>
> julia


Hi Julia,

Tested your suggestion and it works well. Thanks!

Best wishes,
Mats Kindahl

>
>
>
>
>
>>> However, when executing spatch, I get the following error:
>>>
>>>        mats@abzu:~/lang/cocci/postgres$ spatch --all-includes --sp-file heap_tuple.cocci code/heap_tuple.c
>>>        init_defs_builtins: /usr/lib/coccinelle/standard.h
>>>        before [[Tag29 ("{", 0)]; [Tag22 ((Tag1 ((Tag5 (("if", (11, 2, 0, 0, " "), Tag2 (0), 0), ("(", (11, 5, 0, 0, " "), Tag2 (0), 0), (Tag9 ((Tag3 (((
>>>        ((("HeapTupleIsValid", (11, 7, 0, 0, ""), Tag2 (0), 0)), 11, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0)), 11, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0),
>>>        ("(", (11, 23, 0, 0, ""), Tag2 (0), 0), ([(((Tag1 ((("rule starting on line 1", "tuple"), (11, 24, 0, 0, ""), Tag2 (0), 0), 1, 0, 0), 11, 0, 0,
>>>        0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0)), 11, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0)], 11, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0), (")", (11, 29, 0, 0,
>>>        ""), Tag2 (0), 0)), 11, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0), (6, (11, 6, 0, 0, ""), Tag2 (0), 0)), 11, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0),
>>>        (")", (11, 30, 0, 0, ""), Tag2 (0), 0)), 11, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0), (Tag8 ((Tag4 (((Tag3 (((((("elog", (12, 4, 0, 0, "   "), Ta
>>>        g2 (0), 0)), 12, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0)), 12, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0), ("(", (12, 8, 0, 0, ""), Tag2 (0), 0), ([((
>>>        ((("ERROR", (12, 9, 0, 0, ""), Tag2 (0), 0)), 12, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0)), 12, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0); (Tag26 (("
>>>        ,", (12, 14, 0, 0, ""), Tag2 (0), 0)), 12, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0); (Tag1 ((("cache lookup failed for %u", 4), (12, 16, 0, 0, " ")
>>>        , Tag2 (0), 0)), 12, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0); (Tag26 ((",", (12, 44, 0, 0, ""), Tag2 (0), 0)), 12, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
>>>        , 1, 0); (Tag22 ((("rule starting on line 1", "id"), (12, 46, 0, 0, " "), Tag2 (0), 0), 1, 0, 0, 0, 0, 0), 12, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1
>>>        , 0)], 12, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0), (")", (12, 48, 0, 0, ""), Tag2 (0), 0)), 12, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0)), (";", (1
>>>        2, 49, 0, 0, ""), Tag2 (0), 0)), 12, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0)), 12, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0), (0, 0, 0, Tag1 (0, 0)))
>>>        , 11, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0))]]
>>>        context tree should not have bad code before
>>>
>>> First, what is causing the error?
>> I think the + code in the disjunction, based on the (mostly unreadable)
>> complaint.
>>
>>> Second, is the semantic patch supposed to work or is another method preferred?
>> It looks ok to me.  Actually, I have never seen this complaint.  I will
>> look into it.
>>
>> julia