Re: [PATCH v2 1/6] libfdt: Add a subnodes iterator macro
From: David Gibson <hidden>
Date: 2016-07-12 01:52:30
On Mon, Jul 11, 2016 at 09:56:18PM +0200, Maxime Ripard wrote:
From: Thierry Reding <redacted> The fdt_for_each_subnode() iterator macro provided by this patch can be used to iterate over a device tree node's subnodes. At each iteration a loop variable will be set to the next subnode. Signed-off-by: Thierry Reding <redacted> Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Code looks ok, but there are some nits in the comments.
quoted hunk
--- libfdt/libfdt.h | 25 +++++++++++++++++++++++++ tests/subnode_iterate.c | 8 ++------ 2 files changed, 27 insertions(+), 6 deletions(-)diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h index 36222fd4a6f4..0cf46872b54e 100644 --- a/libfdt/libfdt.h +++ b/libfdt/libfdt.h@@ -168,6 +168,31 @@ int fdt_first_subnode(const void *fdt, int offset); */ int fdt_next_subnode(const void *fdt, int offset); +/** + * fdt_for_each_subnode - iterate over all subnodes of a parent
Parameter descriptions should go here.
+ *
+ * This is actually a wrapper around a for loop and would be used like so:
+ *
+ * fdt_for_each_subnode(fdt, node, parent) {Parameter order hasn't been updated for code revisions.
+ * ... + * use node + * ... + * }
One gotcha with this is what happens if there's an error during the
iteration. I suggest adding something like:
if ((node < 0) && (node != -FDT_ERR_NOTFOUND)) {
/* error handling */
}
to the example, to remind users that they should check for this.
+ * + * Note that this is implemented as a macro and node is used as iterator in
s/node/@node/ - makes it stand out as a parameter.
+ * the loop. It should therefore be a locally allocated variable. The parent
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Remove for brevity.
+ * variable on the other hand is never modified, so it can be constant or
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Remove for brevity.
quoted hunk
+ * even a literal. + * + * @node: child node (int) + * @fdt: FDT blob (const void *) + * @parent: parent node (int) + */ +#define fdt_for_each_subnode(node, fdt, parent) \ + for (node = fdt_first_subnode(fdt, parent); \ + node >= 0; \ + node = fdt_next_subnode(fdt, node)) + /**********************************************************************/ /* General functions */ /**********************************************************************/diff --git a/tests/subnode_iterate.c b/tests/subnode_iterate.c index b9f379d5963c..0fb5c901ebd7 100644 --- a/tests/subnode_iterate.c +++ b/tests/subnode_iterate.c@@ -48,9 +48,7 @@ static void test_node(void *fdt, int parent_offset) subnodes = cpu_to_fdt32(*prop); count = 0; - for (offset = fdt_first_subnode(fdt, parent_offset); - offset >= 0; - offset = fdt_next_subnode(fdt, offset)) + fdt_for_each_subnode(offset, fdt, parent_offset) count++; if (count != subnodes) {@@ -65,9 +63,7 @@ static void check_fdt_next_subnode(void *fdt) int offset; int count = 0; - for (offset = fdt_first_subnode(fdt, 0); - offset >= 0; - offset = fdt_next_subnode(fdt, offset)) { + fdt_for_each_subnode(offset, fdt, 0) { test_node(fdt, offset); count++; }
-- 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
Attachments
- signature.asc [application/pgp-signature] 819 bytes