Re: dtc: Flexible tree checking infrastructure (v2)
So, like, the other day David Gibson mumbled: dtc: Flexible tree checking infrastructure Here, at last, is a substantial start on revising dtc's infrastructure for checking the tree; this is the rework I've been saying was necessary practically since dtc was first release. In the new model, we have a table of check structures, each with a name, references to checking functions, and status variables. Each check can (in principle) be individually switched off or on (as either a warning or error). Checks have a list of prerequisites, so if checks need to rely on results from earlier checks to make sense (or even to avoid crashing) they just need to list the relevant other checks there. For now, only the structural checks and the fixups for phandle references are converted to the new mechanism. The rather more involved semantic checks (which is where this new mechanism will really be useful) will have to be converted in future patches. At present, there's no user interface for turning on/off the checks - the -f option now forces output even if error level checks fail. Again, future patches will be needed to add the fine-grained control, but that should be quite straightforward with the infrastructure implemented here. Also adds a testcase for the handling of bad references, which catches a bug encountered while developing this patch. Signed-off-by: David Gibson [EMAIL PROTECTED] While I've Applied this one, it has introduced this: CC dtc.o dtc.c: In function 'main': dtc.c:199: warning: 'structure_ok' is used uninitialized in this function Followup easy patch? Thanks, jdl ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: dtc: Flexible tree checking infrastructure (v2)
On Mon, Nov 26, 2007 at 04:13:32PM -0600, Jon Loeliger wrote: So, like, the other day David Gibson mumbled: dtc: Flexible tree checking infrastructure Here, at last, is a substantial start on revising dtc's infrastructure for checking the tree; this is the rework I've been saying was necessary practically since dtc was first release. In the new model, we have a table of check structures, each with a name, references to checking functions, and status variables. Each check can (in principle) be individually switched off or on (as either a warning or error). Checks have a list of prerequisites, so if checks need to rely on results from earlier checks to make sense (or even to avoid crashing) they just need to list the relevant other checks there. For now, only the structural checks and the fixups for phandle references are converted to the new mechanism. The rather more involved semantic checks (which is where this new mechanism will really be useful) will have to be converted in future patches. At present, there's no user interface for turning on/off the checks - the -f option now forces output even if error level checks fail. Again, future patches will be needed to add the fine-grained control, but that should be quite straightforward with the infrastructure implemented here. Also adds a testcase for the handling of bad references, which catches a bug encountered while developing this patch. Signed-off-by: David Gibson [EMAIL PROTECTED] While I've Applied this one, it has introduced this: CC dtc.o dtc.c: In function 'main': dtc.c:199: warning: 'structure_ok' is used uninitialized in this function Followup easy patch? Crap. For some reason my compiler isn't giving that warning, so I missed that little bug :(. I'm away at the moment, I'll see what I can do. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
dtc: Flexible tree checking infrastructure (v2)
dtc: Flexible tree checking infrastructure
Here, at last, is a substantial start on revising dtc's infrastructure
for checking the tree; this is the rework I've been saying was
necessary practically since dtc was first release.
In the new model, we have a table of check structures, each with a
name, references to checking functions, and status variables. Each
check can (in principle) be individually switched off or on (as either
a warning or error). Checks have a list of prerequisites, so if
checks need to rely on results from earlier checks to make sense (or
even to avoid crashing) they just need to list the relevant other
checks there.
For now, only the structural checks and the fixups for phandle
references are converted to the new mechanism. The rather more
involved semantic checks (which is where this new mechanism will
really be useful) will have to be converted in future patches.
At present, there's no user interface for turning on/off the checks -
the -f option now forces output even if error level checks fail.
Again, future patches will be needed to add the fine-grained control,
but that should be quite straightforward with the infrastructure
implemented here.
Also adds a testcase for the handling of bad references, which catches
a bug encountered while developing this patch.
Signed-off-by: David Gibson [EMAIL PROTECTED]
---
Oops, the original version had a bug: processing a bad reference would
cause dtc to SEGV. Plus the sense was backwards when checking the
value of 'quiet' to suppress errors/warnings. Fixed here and some
other small changes:
- cleanups to reference processing code
- cleanups to error/warning message printing code
- added a tracing mechanism (compile time option) useful for
debugging problems with the checks
Index: dtc/checks.c
===
--- dtc.orig/checks.c 2007-11-22 13:03:05.0 +1100
+++ dtc/checks.c2007-11-22 14:02:26.0 +1100
@@ -20,104 +20,293 @@
#include dtc.h
-/*
- * Structural check functions
- */
+#ifdef TRACE_CHECKS
+#define TRACE(c, ...) \
+ do { \
+ fprintf(stderr, === %s: , (c)-name); \
+ fprintf(stderr, __VA_ARGS__); \
+ fprintf(stderr, \n); \
+ } while (0)
+#else
+#define TRACE(c, fmt, ...) do { } while (0)
+#endif
+
+enum checklevel {
+ IGNORE = 0,
+ WARN = 1,
+ ERROR = 2,
+};
-#define ERRMSG(...) if (quiet 2) fprintf(stderr, ERROR: __VA_ARGS__)
-#define WARNMSG(...) if (quiet 1) fprintf(stderr, Warning: __VA_ARGS__)
+enum checkstatus {
+ UNCHECKED = 0,
+ PREREQ,
+ PASSED,
+ FAILED,
+};
-#define DO_ERR(...) do {ERRMSG(__VA_ARGS__); ok = 0; } while (0)
+struct check;
-static int check_names(struct node *tree)
-{
- struct node *child, *child2;
- struct property *prop, *prop2;
- int len = strlen(tree-name);
- int ok = 1;
+typedef void (*tree_check_fn)(struct check *c, struct node *dt);
+typedef void (*node_check_fn)(struct check *c, struct node *dt, struct node
*node);
+typedef void (*prop_check_fn)(struct check *c, struct node *dt,
+ struct node *node, struct property *prop);
+
+struct check {
+ const char *name;
+ tree_check_fn tree_fn;
+ node_check_fn node_fn;
+ prop_check_fn prop_fn;
+ void *data;
+ enum checklevel level;
+ enum checkstatus status;
+ int inprogress;
+ int num_prereqs;
+ struct check **prereq;
+};
- if (len == 0 tree-parent)
- DO_ERR(Empty, non-root nodename at %s\n, tree-fullpath);
+#define CHECK(nm, tfn, nfn, pfn, d, lvl, ...) \
+ static struct check *nm##_prereqs[] = { __VA_ARGS__ }; \
+ static struct check nm = { \
+ .name = #nm, \
+ .tree_fn = (tfn), \
+ .node_fn = (nfn), \
+ .prop_fn = (pfn), \
+ .data = (d), \
+ .level = (lvl), \
+ .status = UNCHECKED, \
+ .num_prereqs = ARRAY_SIZE(nm##_prereqs), \
+ .prereq = nm##_prereqs, \
+ };
+
+#define TREE_CHECK(nm, d, lvl, ...) \
+ CHECK(nm, check_##nm, NULL, NULL, d, lvl, __VA_ARGS__)
+#define NODE_CHECK(nm, d, lvl, ...) \
+ CHECK(nm, NULL, check_##nm, NULL, d, lvl, __VA_ARGS__)
+#define PROP_CHECK(nm, d, lvl, ...) \
+ CHECK(nm, NULL, NULL, check_##nm, d, lvl, __VA_ARGS__)
+#define BATCH_CHECK(nm, lvl, ...) \
+ CHECK(nm, NULL, NULL, NULL, NULL, lvl, __VA_ARGS__)
+
+static inline void check_msg(struct check *c, const char *fmt, ...)
+{
+ va_list ap;
+ va_start(ap, fmt);
+
+ if ((c-level WARN) || (c-level = quiet))
+ return; /* Suppress message */
+
+ fprintf(stderr, %s (%s): ,
+ (c-level == ERROR) ? ERROR : Warning, c-name);
+ vfprintf(stderr, fmt, ap);
+ fprintf(stderr, \n);
+}
- if (len
dtc: Flexible tree checking infrastructure
Here, at last, is a substantial start on revising dtc's infrastructure
for checking the tree; this is the rework I've been saying was
necessary practically since dtc was first release.
In the new model, we have a table of check structures, each with a
name, references to checking functions, and status variables. Each
check can (in principle) be individually switched off or on (as either
a warning or error). Checks have a list of prerequisites, so if
checks need to rely on results from earlier checks to make sense (or
even to avoid crashing) they just need to list the relevant other
checks there.
For now, only the structural checks and the fixups for phandle
references are converted to the new mechanism. The rather more
involved semantic checks (which is where this new mechanism will
really be useful) will have to be converted in future patches.
At present, there's no user interface for turning on/off the checks -
the -f option now forces output even if error level checks fail.
Again, future patches will be needed to add the fine-grained control,
but that should be quite straightforward with the infrastructure
implemented here.
Signed-off-by: David Gibson [EMAIL PROTECTED]
Index: dtc/checks.c
===
--- dtc.orig/checks.c 2007-11-21 12:27:13.0 +1100
+++ dtc/checks.c2007-11-21 16:28:08.0 +1100
@@ -20,104 +20,281 @@
#include dtc.h
-/*
- * Structural check functions
- */
+enum checklevel {
+ IGNORE = 0,
+ WARN = 1,
+ ERROR = 2,
+};
-#define ERRMSG(...) if (quiet 2) fprintf(stderr, ERROR: __VA_ARGS__)
-#define WARNMSG(...) if (quiet 1) fprintf(stderr, Warning: __VA_ARGS__)
+enum checkstatus {
+ UNCHECKED = 0,
+ PREREQ,
+ PASSED,
+ FAILED,
+};
-#define DO_ERR(...) do {ERRMSG(__VA_ARGS__); ok = 0; } while (0)
+struct check;
+
+typedef void (*tree_check_fn)(struct check *c, struct node *dt);
+typedef void (*node_check_fn)(struct check *c, struct node *dt, struct node
*node);
+typedef void (*prop_check_fn)(struct check *c, struct node *dt,
+ struct node *node, struct property *prop);
+
+struct check {
+ const char *name;
+ tree_check_fn tree_fn;
+ node_check_fn node_fn;
+ prop_check_fn prop_fn;
+ void *data;
+ enum checklevel level;
+ enum checkstatus status;
+ int inprogress;
+ int num_prereqs;
+ struct check **prereq;
+};
-static int check_names(struct node *tree)
+#define CHECK(nm, tfn, nfn, pfn, d, lvl, ...) \
+ static struct check *nm##_prereqs[] = { __VA_ARGS__ }; \
+ static struct check nm = { \
+ .name = #nm, \
+ .tree_fn = (tfn), \
+ .node_fn = (nfn), \
+ .prop_fn = (pfn), \
+ .data = (d), \
+ .level = (lvl), \
+ .status = UNCHECKED, \
+ .num_prereqs = ARRAY_SIZE(nm##_prereqs), \
+ .prereq = nm##_prereqs, \
+ };
+
+#define TREE_CHECK(nm, d, lvl, ...) \
+ CHECK(nm, check_##nm, NULL, NULL, d, lvl, __VA_ARGS__)
+#define NODE_CHECK(nm, d, lvl, ...) \
+ CHECK(nm, NULL, check_##nm, NULL, d, lvl, __VA_ARGS__)
+#define PROP_CHECK(nm, d, lvl, ...) \
+ CHECK(nm, NULL, NULL, check_##nm, d, lvl, __VA_ARGS__)
+#define BATCH_CHECK(nm, lvl, ...) \
+ CHECK(nm, NULL, NULL, NULL, NULL, lvl, __VA_ARGS__)
+
+#define CHECKMSG(c, fmt, ...) \
+ do { \
+ if (quiet (c)-level) \
+ ; /* Suppress message */ \
+ else if ((c)-level == ERROR) \
+ fprintf(stderr, ERROR (%s): fmt \n, \
+ (c)-name, __VA_ARGS__); \
+ else if ((c)-level == WARN) \
+ fprintf(stderr, Warning (%s): fmt \n, \
+ (c)-name, __VA_ARGS__); \
+ } while (0)
+
+#define FAIL(c, fmt, ...) \
+ do { \
+ (c)-status = FAILED; \
+ CHECKMSG((c), fmt, __VA_ARGS__); \
+ } while (0)
+
+static void check_nodes_props(struct check *c, struct node *dt, struct node
*node)
{
- struct node *child, *child2;
- struct property *prop, *prop2;
- int len = strlen(tree-name);
- int ok = 1;
+ struct node *child;
+ struct property *prop;
- if (len == 0 tree-parent)
- DO_ERR(Empty, non-root nodename at %s\n, tree-fullpath);
+ if (c-node_fn)
+ c-node_fn(c, dt, node);
- if (len MAX_NODENAME_LEN)
- WARNMSG(Overlength nodename at %s\n, tree-fullpath);
+ if (c-prop_fn)
+ for_each_property(node, prop)
+ c-prop_fn(c, dt, node, prop);
- for_each_property(tree, prop) {
- /* check for duplicates */
- /* FIXME: do this more efficiently */
- for (prop2 = prop-next; prop2; prop2 = prop2-next) {
-
