[DTrace-devel] [PATCH] Clean up argument processing (and its testing) for clear()
Kris Van Hees
kris.van.hees at oracle.com
Wed Jan 20 10:18:26 PST 2021
Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>
On Fri, Jan 15, 2021 at 11:39:20PM -0500, eugene.loh at oracle.com wrote:
> From: Eugene Loh <eugene.loh at oracle.com>
>
> Arguments to clear() are vetted in dt_idcook_sign(). Therefore,
> remove such vetting from dt_action_clear(). In dt_cg_act_clear(),
> the arguments do not need to be vetted, but write the action to
> the output buffer in anticipation of a future patch that will
> implement clear() on the consumer side.
>
> Clean up associated testing:
>
> - While err.D_CLEAR_AGGARG.bad.d checks processing of the
> required clear() argument, the test is unnecessarily
> complicated (and relies on not-yet-implemented features)
> and its naming is not consistent with [de]normalize tests.
> Replace it with err.D_CLEAR_AGGARG.clear_no_agg.d.
>
> - There are similar issues with err.D_CLEAR_PROTO.bad.d.
> Further, the expected error has changed.
> Replace the test with err.D_PROTO_LEN.clear_too_few_args.d.
>
> - Add err.D_PROTO_LEN.clear_too_many_args.d.
>
> - Clean up err.D_PROTO_LEN.denorm_too_few_args.d a little
> to continue the work done on denormalize() tests and to
> maintain similarity between denormalize() and clear() tests.
>
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> ---
> libdtrace/dt_cc.c | 16 +-----
> libdtrace/dt_cg.c | 13 ++---
> test/unittest/aggs/err.D_CLEAR_AGGARG.bad.d | 55 -------------------
> .../aggs/err.D_CLEAR_AGGARG.clear_no_agg.d | 18 ++++++
> test/unittest/aggs/err.D_CLEAR_PROTO.bad.d | 55 -------------------
> .../aggs/err.D_PROTO_LEN.clear_too_few_args.d | 18 ++++++
> .../err.D_PROTO_LEN.clear_too_many_args.d | 19 +++++++
> .../err.D_PROTO_LEN.denorm_too_few_args.d | 1 -
> 8 files changed, 63 insertions(+), 132 deletions(-)
> delete mode 100644 test/unittest/aggs/err.D_CLEAR_AGGARG.bad.d
> create mode 100644 test/unittest/aggs/err.D_CLEAR_AGGARG.clear_no_agg.d
> delete mode 100644 test/unittest/aggs/err.D_CLEAR_PROTO.bad.d
> create mode 100644 test/unittest/aggs/err.D_PROTO_LEN.clear_too_few_args.d
> create mode 100644 test/unittest/aggs/err.D_PROTO_LEN.clear_too_many_args.d
>
> diff --git a/libdtrace/dt_cc.c b/libdtrace/dt_cc.c
> index e8865c45..1d111fa6 100644
> --- a/libdtrace/dt_cc.c
> +++ b/libdtrace/dt_cc.c
> @@ -294,34 +294,22 @@ dt_action_clear(dtrace_hdl_t *dtp, dt_node_t *dnp, dtrace_stmtdesc_t *sdp)
> dt_node_t *anp;
>
> char n[DT_TYPE_NAMELEN];
> - int argc = 0;
> -
> - for (anp = dnp->dn_args; anp != NULL; anp = anp->dn_list)
> - argc++; /* count up arguments for error messages below */
> -
> - if (argc != 1) {
> - dnerror(dnp, D_CLEAR_PROTO,
> - "%s( ) prototype mismatch: %d args passed, 1 expected\n",
> - dnp->dn_ident->di_name, argc);
> - }
>
> anp = dnp->dn_args;
> assert(anp != NULL);
>
> - if (anp->dn_kind != DT_NODE_AGG) {
> + if (anp->dn_kind != DT_NODE_AGG)
> dnerror(dnp, D_CLEAR_AGGARG,
> "%s( ) argument #1 is incompatible with prototype:\n"
> "\tprototype: aggregation\n\t argument: %s\n",
> dnp->dn_ident->di_name,
> dt_node_type_name(anp, n, sizeof(n)));
> - }
>
> aid = anp->dn_ident;
>
> - if (aid->di_gen == dtp->dt_gen && !(aid->di_flags & DT_IDFLG_MOD)) {
> + if (aid->di_gen == dtp->dt_gen && !(aid->di_flags & DT_IDFLG_MOD))
> dnerror(dnp, D_CLEAR_AGGBAD,
> "undefined aggregation: @%s\n", aid->di_name);
> - }
>
> ap = dt_stmt_action(dtp, sdp);
> dt_action_difconst(ap, anp->dn_ident->di_id, DTRACEACT_LIBACT);
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 2753eeac..05298813 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -545,26 +545,24 @@ dt_cg_act_chill(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind)
> static void
> dt_cg_act_clear(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind)
> {
> - dt_node_t *anp;
> - dt_ident_t *aid;
> - char n[DT_TYPE_NAMELEN];
> + dt_node_t *anp;
> + dt_ident_t *aid;
> + char n[DT_TYPE_NAMELEN];
>
> anp = dnp->dn_args;
> assert(anp != NULL);
> - if (anp->dn_kind != DT_NODE_AGG) {
> + if (anp->dn_kind != DT_NODE_AGG)
> dnerror(dnp, D_CLEAR_AGGARG,
> "%s( ) argument #1 is incompatible with prototype:\n"
> "\tprototype: aggregation\n\t argument: %s\n",
> dnp->dn_ident->di_name,
> dt_node_type_name(anp, n, sizeof(n)));
> - }
>
> aid = anp->dn_ident;
> if (aid->di_gen == pcb->pcb_hdl->dt_gen &&
> - !(aid->di_flags & DT_IDFLG_MOD)) {
> + !(aid->di_flags & DT_IDFLG_MOD))
> dnerror(dnp, D_CLEAR_AGGBAD,
> "undefined aggregation: @%s\n", aid->di_name);
> - }
>
> /*
> * FIXME: Needs implementation
> @@ -572,6 +570,7 @@ dt_cg_act_clear(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind)
> * DEPENDS ON: How aggregations are implemented using eBPF (hashmap?).
> * AGGID = aid->di_id
> */
> + dt_cg_store_val(pcb, anp, DTRACEACT_LIBACT, NULL, DT_ACT_CLEAR);
> }
>
> /*
> diff --git a/test/unittest/aggs/err.D_CLEAR_AGGARG.bad.d b/test/unittest/aggs/err.D_CLEAR_AGGARG.bad.d
> deleted file mode 100644
> index 655ee0df..00000000
> --- a/test/unittest/aggs/err.D_CLEAR_AGGARG.bad.d
> +++ /dev/null
> @@ -1,55 +0,0 @@
> -/*
> - * Oracle Linux DTrace.
> - * Copyright (c) 2006, Oracle and/or its affiliates. All rights reserved.
> - * Licensed under the Universal Permissive License v 1.0 as shown at
> - * http://oss.oracle.com/licenses/upl.
> - */
> -/* @@xfail: dtv2 */
> -
> -/*
> - * ASSERTION:
> - * The argument to clear() must be an aggregation.
> - *
> - * SECTION: Aggregations/Clearing aggregations
> - *
> - *
> - */
> -
> -#pragma D option quiet
> -#pragma D option aggrate=10ms
> -#pragma D option switchrate=10ms
> -
> -BEGIN
> -{
> - i = 0;
> - start = timestamp;
> -}
> -
> -tick-100ms
> -/i < 20/
> -{
> - @func[i%5] = sum(i * 100);
> - i++;
> -}
> -
> -tick-100ms
> -/i == 10/
> -{
> - printf("Aggregation data before clear():\n");
> - printa(@func);
> -
> - clear(count());
> -
> - printf("Aggregation data after clear():\n");
> - printa(@func);
> - i++;
> -}
> -
> -tick-100ms
> -/i == 20/
> -{
> - printf("Final aggregation data:\n");
> - printa(@func);
> -
> - exit(0);
> -}
> diff --git a/test/unittest/aggs/err.D_CLEAR_AGGARG.clear_no_agg.d b/test/unittest/aggs/err.D_CLEAR_AGGARG.clear_no_agg.d
> new file mode 100644
> index 00000000..d60e67ea
> --- /dev/null
> +++ b/test/unittest/aggs/err.D_CLEAR_AGGARG.clear_no_agg.d
> @@ -0,0 +1,18 @@
> +/*
> + * Oracle Linux DTrace.
> + * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
> + * Licensed under the Universal Permissive License v 1.0 as shown at
> + * http://oss.oracle.com/licenses/upl.
> + */
> +
> +/*
> + * ASSERTION: The argument to clear() must be an aggregation.
> + *
> + * SECTION: Aggregations/Clearing aggregations
> + */
> +
> +BEGIN
> +{
> + clear(count());
> + exit(0);
> +}
> diff --git a/test/unittest/aggs/err.D_CLEAR_PROTO.bad.d b/test/unittest/aggs/err.D_CLEAR_PROTO.bad.d
> deleted file mode 100644
> index a6d92860..00000000
> --- a/test/unittest/aggs/err.D_CLEAR_PROTO.bad.d
> +++ /dev/null
> @@ -1,55 +0,0 @@
> -/*
> - * Oracle Linux DTrace.
> - * Copyright (c) 2006, Oracle and/or its affiliates. All rights reserved.
> - * Licensed under the Universal Permissive License v 1.0 as shown at
> - * http://oss.oracle.com/licenses/upl.
> - */
> -/* @@xfail: dtv2 */
> -
> -/*
> - * ASSERTION:
> - * clear() should handle no args as an error.
> - *
> - * SECTION: Aggregations/Clearing aggregations
> - *
> - *
> - */
> -
> -#pragma D option quiet
> -#pragma D option aggrate=10ms
> -#pragma D option switchrate=10ms
> -
> -BEGIN
> -{
> - i = 0;
> - start = timestamp;
> -}
> -
> -tick-100ms
> -/i < 20/
> -{
> - @func[i%5] = sum(i * 100);
> - i++;
> -}
> -
> -tick-100ms
> -/i == 10/
> -{
> - printf("Aggregation data before clear():\n");
> - printa(@func);
> -
> - clear();
> -
> - printf("Aggregation data after clear():\n");
> - printa(@func);
> - i++;
> -}
> -
> -tick-100ms
> -/i == 20/
> -{
> - printf("Final aggregation data:\n");
> - printa(@func);
> -
> - exit(0);
> -}
> diff --git a/test/unittest/aggs/err.D_PROTO_LEN.clear_too_few_args.d b/test/unittest/aggs/err.D_PROTO_LEN.clear_too_few_args.d
> new file mode 100644
> index 00000000..112081ec
> --- /dev/null
> +++ b/test/unittest/aggs/err.D_PROTO_LEN.clear_too_few_args.d
> @@ -0,0 +1,18 @@
> +/*
> + * Oracle Linux DTrace.
> + * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
> + * Licensed under the Universal Permissive License v 1.0 as shown at
> + * http://oss.oracle.com/licenses/upl.
> + */
> +
> +/*
> + * ASSERTION: The clear() action requires one argument (agg).
> + *
> + * SECTION: Aggregations/Data Normalization
> + */
> +
> +BEGIN
> +{
> + clear();
> + exit(0);
> +}
> diff --git a/test/unittest/aggs/err.D_PROTO_LEN.clear_too_many_args.d b/test/unittest/aggs/err.D_PROTO_LEN.clear_too_many_args.d
> new file mode 100644
> index 00000000..d5fd70e8
> --- /dev/null
> +++ b/test/unittest/aggs/err.D_PROTO_LEN.clear_too_many_args.d
> @@ -0,0 +1,19 @@
> +/*
> + * Oracle Linux DTrace.
> + * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
> + * Licensed under the Universal Permissive License v 1.0 as shown at
> + * http://oss.oracle.com/licenses/upl.
> + */
> +
> +/*
> + * ASSERTION: The clear() action accepts only one argument (agg).
> + *
> + * SECTION: Aggregations/Data Normalization
> + */
> +
> +BEGIN
> +{
> + @a = count();
> + clear(@a, 1);
> + exit(0);
> +}
> diff --git a/test/unittest/aggs/err.D_PROTO_LEN.denorm_too_few_args.d b/test/unittest/aggs/err.D_PROTO_LEN.denorm_too_few_args.d
> index c2854d39..4ad9d33f 100644
> --- a/test/unittest/aggs/err.D_PROTO_LEN.denorm_too_few_args.d
> +++ b/test/unittest/aggs/err.D_PROTO_LEN.denorm_too_few_args.d
> @@ -13,7 +13,6 @@
>
> BEGIN
> {
> - @a = count();
> denormalize();
> exit(0);
> }
> --
> 2.18.4
>
>
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel
More information about the DTrace-devel
mailing list