[DTrace-devel] [PATCH 01/14] Have dt_cg_load_scalar report runtime error if it fails

Nick Alcock nick.alcock at oracle.com
Tue May 2 15:25:48 UTC 2023


On 2 May 2023, eugene loh said:

> From: Eugene Loh <eugene.loh at oracle.com>

Much needed, if you ask me. (I really should go through and add a lot
more error checking to DTrace's CTF operations as well. Too many things
which can fail are assumed to be impossible to fail...)

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

(with a couple of stylistic nits below)

> -	dt_regset_free(drp, BPF_REG_0);
>  	dt_regset_free_args(drp);
> +
> +        /* check if we were successful */
> +	emit(dlp,  BPF_BRANCH_IMM(BPF_JEQ, BPF_REG_0, 0, Lokay));
> +	dt_regset_free(drp, BPF_REG_0);
> +	dt_cg_probe_error(yypcb, DTRACEFLT_BADADDR, DT_ISREG, dnp->dn_reg);
> +	emitl(dlp, Lokay,
> +		   BPF_NOP());
> +
> +	/* load the copy of the data in "safe" memory (D stack) */

The code looks fine, but DTrace comments are of the style

/*
 * Load the copy of the data in "safe" memory (D stack).
 */

I happen to think this is annoyingly verbose and needlessly wastes
vertical space, discouraging comments, and would be very happy to move
to a scheme where one-line (or one paragraph?) comments are of the style

/* Load the copy of the data in "safe" memory (D stack). */

But at the very least I think we want a capital letter :)

Nice tests!

-- 
NULL && (void)



More information about the DTrace-devel mailing list