Re: [cocci] Checking evaluation of another SmPL script

On Mon, 25 May 2026, Markus Elfring wrote:

> >> We are trying to clarify different expectations for SmPL data processing
> >> with goto statements.
> > Please actually clarify your expectations. …
> Would you find the following test result “reasonable”?
>
> https://elixir.bootlin.com/linux/v7.1-rc4/source/fs/nfs/pnfs.c#L2128-L2385
>
> Markus_Elfring@Sonne:…/Projekte/Linux/next-analyses> time /usr/bin/spatch --show-ctl-text --no-gotos …/Projekte/Coccinelle/janitor/show_UAF_1.cocci fs/nfs/pnfs.c
> …
> @display@
> identifier i;
> expression e;
> @@
>
> *pnfs_put_layout_hdr*(*i*)*;
> ...   WHEN != i = e
>
> *trace_pnfs_update_layout*(*...*, *i*, *...*)
>
>
> CTL =
>
> (!EX(EX(EX(Exit))) &,
>  (Ex_ i .
>   ((Ex _v . pnfs_put_layout_hdr(i);) &,
>    EX(E[(!After &
>          (!(pnfs_put_layout_hdr(i); v
>             InnerAnd(trace_pnfs_update_layout(..., i, ...)))
>           & !(After v InnerAnd((Ex_ e . i = e)))))
>       U InnerAnd((Ex _v . trace_pnfs_update_layout(..., i, ...)))]))))
>
> diff =
> …
> @@ -2216,7 +2216,6 @@ lookup_again:
>                                            TASK_KILLABLE));
>                 if (IS_ERR(lseg))
>                         goto out_put_layout_hdr;
> -               pnfs_put_layout_hdr(lo);
>                 goto lookup_again;
>         }
>
> @@ -2229,22 +2228,14 @@ lookup_again:
>                 dprintk("%s wait for layoutreturn\n", __func__);
>                 lseg = ERR_PTR(pnfs_prepare_to_retry_layoutget(lo));
>                 if (!IS_ERR(lseg)) {
> -                       pnfs_put_layout_hdr(lo);
>                         dprintk("%s retrying\n", __func__);
> -                       trace_pnfs_update_layout(ino, pos, count, iomode, lo,
> -                                                lseg,
> -                                                PNFS_UPDATE_LAYOUT_RETRY);
>                         goto lookup_again;
>                 }
> -               trace_pnfs_update_layout(ino, pos, count, iomode, lo, lseg,
> -                                        PNFS_UPDATE_LAYOUT_RETURN);

When you use * there s no way to go what matches with what.  But this line
looks questionable because there isn't an obvious path from the
pnfs_put_layout_hdr above to the trace_pnfs_update_layout here.  You get
this because of the no-gotos flag not taking the goto lookup_again;

julia


>                 goto out_put_layout_hdr;
>         }
>
>         lseg = pnfs_find_lseg(lo, &arg, strict_iomode);
>         if (lseg) {
> -               trace_pnfs_update_layout(ino, pos, count, iomode, lo, lseg,
> -                               PNFS_UPDATE_LAYOUT_FOUND_CACHED);
>                 goto out_unlock;
>         }
>
> @@ -2268,7 +2259,6 @@ lookup_again:
>                                                 TASK_KILLABLE));
>                         if (IS_ERR(lseg))
>                                 goto out_put_layout_hdr;
> -                       pnfs_put_layout_hdr(lo);
>                         dprintk("%s retrying\n", __func__);
>                         goto lookup_again;
>                 }
> @@ -2280,12 +2270,8 @@ lookup_again:
>                                         NULL, &stateid, NULL);
>                 if (status != 0) {
>                         lseg = ERR_PTR(status);
> -                       trace_pnfs_update_layout(ino, pos, count,
> -                                       iomode, lo, lseg,
> -                                       PNFS_UPDATE_LAYOUT_INVALID_OPEN);
>                         nfs4_schedule_stateid_recovery(server, ctx->state);
>                         pnfs_clear_first_layoutget(lo);
> -                       pnfs_put_layout_hdr(lo);
>                         goto lookup_again;
>                 }
>                 spin_lock(&ino->i_lock);
> @@ -2294,8 +2280,6 @@ lookup_again:
>         }
>
>         if (pnfs_layoutgets_blocked(lo)) {
> -               trace_pnfs_update_layout(ino, pos, count, iomode, lo, lseg,
> -                               PNFS_UPDATE_LAYOUT_BLOCKED);
>                 goto out_unlock;
>         }
>         nfs_layoutget_begin(lo);
> @@ -2314,8 +2298,6 @@ lookup_again:
>         lgp = pnfs_alloc_init_layoutget_args(ino, ctx, &stateid, &arg, gfp_flags);
>         if (!lgp) {
>                 lseg = ERR_PTR(-ENOMEM);
> -               trace_pnfs_update_layout(ino, pos, count, iomode, lo, NULL,
> -                                        PNFS_UPDATE_LAYOUT_NOMEM);
>                 nfs_layoutget_end(lo);
>                 goto out_put_layout_hdr;
>         }
> @@ -2324,8 +2306,6 @@ lookup_again:
>         pnfs_get_layout_hdr(lo);
>
>         lseg = nfs4_proc_layoutget(lgp, &exception);
> -       trace_pnfs_update_layout(ino, pos, count, iomode, lo, lseg,
> -                                PNFS_UPDATE_LAYOUT_SEND_LAYOUTGET);
>         nfs_layoutget_end(lo);
>         if (IS_ERR(lseg)) {
>                 switch(PTR_ERR(lseg)) {
> @@ -2356,7 +2336,6 @@ lookup_again:
>                                 pnfs_clear_first_layoutget(lo);
>                         trace_pnfs_update_layout(ino, pos, count,
>                                 iomode, lo, lseg, PNFS_UPDATE_LAYOUT_RETRY);
> -                       pnfs_put_layout_hdr(lo);
>                         goto lookup_again;
>                 }
>         } else {
> @@ -2366,8 +2345,6 @@ lookup_again:
>  out_put_layout_hdr:
>         if (first)
>                 pnfs_clear_first_layoutget(lo);
> -       trace_pnfs_update_layout(ino, pos, count, iomode, lo, lseg,
> -                                PNFS_UPDATE_LAYOUT_EXIT);
>         pnfs_put_layout_hdr(lo);
>  out:
>         dprintk("%s: inode %s/%llu pNFS layout segment %s for "
>
> real    0m1,038s
> user    0m0,997s
> sys     0m0,036s
>
>
>
> Are such technical details still questionable?
>
> Regards,
> Markus
>