[DTrace-devel] [PATCH 2/3] Clean up argument processing (and its testing) for clear()
eugene.loh at oracle.com
eugene.loh at oracle.com
Thu Feb 11 16:25:34 PST 2021
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>
Reviewed-by: Kris Van Hees <kris.van.hees 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 30c8b23f..310d687c 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
More information about the DTrace-devel
mailing list