Re: [PATCH 3/4] cg: reject clauses using return() by default

Nick Alcock
2025-07-15T10:37:51Z

On 15 Jul 2025, Kris Van Hees verbalised:

> Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>

Reviewed-by: Nick Alcock <nick.alcock@oracle.com>

> ---
> libdtrace/dt_cg.c | 10 +++++++++-
> libdtrace/dt_errtags.h | 1 +
> libdtrace/dtrace.h | 19 ++++++++++---------
> 3 files changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 738597ed..0607cd4e 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -875,6 +875,10 @@ dt_cg_call_clause(dtrace_hdl_t *dtp, dtrace_stmtdesc_t *sdp, dt_clause_arg_t *ar
> */
> if (prp->prov->impl->reject_clause != NULL)
> prp->prov->impl->reject_clause(prp, sdp->dtsd_clauseflags);
> + else if (sdp->dtsd_clauseflags & DT_CLSFLAG_RETURN)
> + xyerror(D_ACT_RETURN, "return() not allowed for %s:%s:%s:%s\n",
> + prp->desc->prv, prp->desc->mod, prp->desc->fun,
> + prp->desc->prb);

Ah, I guess these are the "default checks" given in the previous commit,
so you can just set up the reject_clause() if destructive mode is on,
and not need to check here at all.

> diff --git a/libdtrace/dtrace.h b/libdtrace/dtrace.h
> index 20672d3f..f0c1b8dc 100644
> --- a/libdtrace/dtrace.h
> +++ b/libdtrace/dtrace.h
> @@ -154,15 +154,16 @@ typedef struct dtrace_stmtdesc {
> } dtrace_stmtdesc_t;
>
> /* dtsd clause flags */
> -#define DT_CLSFLAG_DATAREC 1 /* data-recording */
> -#define DT_CLSFLAG_SPECULATE 2 /* speculate */
> -#define DT_CLSFLAG_COMMIT 4 /* commit */
> -#define DT_CLSFLAG_COMMIT_DISCARD 8 /* commit/discard */
> -#define DT_CLSFLAG_EXIT 16 /* exit */
> -#define DT_CLSFLAG_DESTRUCT 32 /* destructive */
> -#define DT_CLSFLAG_AGGREGATION 64 /* aggregation */
> -#define DT_CLSFLAG_USDT_INCLUDE 128 /* could be used in USDT clause */
> -#define DT_CLSFLAG_USDT_EXCLUDE 256 /* could not be used in USDT clause */
> +#define DT_CLSFLAG_DATAREC 0x0001 /* data-recording */
> +#define DT_CLSFLAG_SPECULATE 0x0002 /* speculate */
> +#define DT_CLSFLAG_COMMIT 0x0004 /* commit */
> +#define DT_CLSFLAG_COMMIT_DISCARD 0x0008 /* commit/discard */
> +#define DT_CLSFLAG_EXIT 0x0010 /* exit */
> +#define DT_CLSFLAG_DESTRUCT 0x0020 /* destructive */
> +#define DT_CLSFLAG_RETURN 0x0040 /* aggregation */
> +#define DT_CLSFLAG_AGGREGATION 0x0080 /* return action */
> +#define DT_CLSFLAG_USDT_INCLUDE 0x0100 /* could be used in USDT clause */
> +#define DT_CLSFLAG_USDT_EXCLUDE 0x0200 /* could not be used in USDT clause */

Oh yes please! I mean we all know our powers of two anyway, but this is
just cleaner. (Not sure why you stuffed DT_CLSFLAG_RETURN in the middle
like that, though.)

--
NULL && (void)