[PATCH RFC 2/5] dtc: livetree: Add more tree parsing helpers

On Thu, Feb 20, 2014 at 07:06:48PM +0100, Tomasz Figa wrote:

1) using Unix error codes as return values is a convention which is
not currently used anywhere in dtc.  Introducing it in the middle of
other changes is not a good idea.

2) Surely calling this on a NULL property is a bug in the caller, so
it should be an assert().

+	if (!prop->val.val)
+		return -ENODATA;

An empty property is a valid stringlist with 0 entries, so this should
return 0 not an error.

+	if (strnlen(prop->val.val, prop->val.len) >= prop->val.len)
+		return -EILSEQ;
+
+	p = prop->val.val;
+
+	for (i = 0; total < prop->val.len; total += l, p += l, i++)
+		l = strlen(p) + 1;
+
+	return i;
+}
+
+const char *propval_next_string(struct property *prop, const char *cur)
+{
+	const char *curv = cur;
+
+	if (!prop)
+		return NULL;
+
+	if (!cur)
+		return prop->val.val;
+
+	curv += strlen(cur) + 1;
+	if (curv >= prop->val.val + prop->val.len)
+		return NULL;
+
+	return curv;
+}
+
+int propval_match_string(struct property *prop, const char *string)
+{
+	size_t l;
+	int i;
+	const char *p, *end;
+
+	if (!prop)
+		return -EINVAL;
+	if (!prop->val.val)
+		return -ENODATA;
+
+	p = prop->val.val;
+	end = p + prop->val.len;
+
+	for (i = 0; p < end; i++, p += l) {
+		l = strlen(p) + 1;
+		if (p + l > end)
+			return -EILSEQ;
+		if (strcmp(string, p) == 0)
+			return i; /* Found it; return index */
+	}
+	return -ENODATA;
+}
+
+static int parse_phandle_with_args(struct node *root,
+					struct property *prop,
+					const char *cells_name,
+					int cell_count, int index,
+					struct of_phandle_args *out_args)
+{
+	const cell_t *list, *list_end;
+	int rc = 0, size, cur_index = 0;
+	uint32_t count = 0;
+	struct node *node = NULL;
+	cell_t phandle;
+
+	/* Retrieve the phandle list property */
+	size = prop->val.len;
+	list = (const cell_t *)prop->val.val;
+	if (!list)
+		return -ENOENT;
+	list_end = list + size / sizeof(*list);
+
+	/* Loop over the phandles until all the requested entry is found */
+	while (list < list_end) {
+		rc = -EINVAL;
+		count = 0;
+
+		/*
+		 * If phandle is 0, then it is an empty entry with no
+		 * arguments.  Skip forward to the next entry.
+		 */
+		phandle = fdt32_to_cpu(*list++);
+		if (phandle) {

Using continue would avoid extraneous indentation and make this easier
to read.

+			/*
+			 * Find the provider node and parse the #*-cells
+			 * property to determine the argument length.
+			 *
+			 * This is not needed if the cell count is hard-coded
+			 * (i.e. cells_name not set, but cell_count is set),
+			 * except when we're going to return the found node
+			 * below.
+			 */
+			if (cells_name || cur_index == index) {
+				node = get_node_by_phandle(root, phandle);
+				if (!node) {
+					pr_err("could not find phandle %u\n",
+						phandle);
+					goto err;
+				}
+			}
+
+			if (cells_name) {
+				struct property *cells_prop;
+
+				cells_prop = get_property(node, cells_name);
+				if (!cells_prop) {
+					pr_err("could not get %s for %s\n",
+						cells_name,
+						node->fullpath);
+					goto err;
+				}
+
+				count = propval_cell(cells_prop);
+			} else {
+				count = cell_count;
+			}
+
+			/*
+			 * Make sure that the arguments actually fit in the
+			 * remaining property data length
+			 */
+			if (list + count > list_end) {
+				pr_err("arguments longer than property\n");
+				goto err;
+			}
+		}
+
+		/*
+		 * All of the error cases above bail out of the loop, so at
+		 * this point, the parsing is successful. If the requested
+		 * index matches, then fill the out_args structure and return,
+		 * or return -ENOENT for an empty entry.
+		 */
+		rc = -ENOENT;
+		if (cur_index == index) {
+			if (!phandle)
+				goto err;
+
+			if (out_args) {
+				int i;
+				if (count > MAX_PHANDLE_ARGS) {
+					pr_warn("argument count higher than MAX_PHANDLE_ARGS\n");
+					count = MAX_PHANDLE_ARGS;

That's more than a warn.  If you've run out of space here that pretty
much has to be a fatal error.

+				}
+				out_args->np = node;
+				out_args->args_count = count;
+				for (i = 0; i < count; i++)
+					out_args->args[i] = fdt32_to_cpu(*list++);
+			}
+
+			/* Found it! return success */
+			return 0;
+		}
+
+		node = NULL;
+		list += count;
+		cur_index++;
+	}
+
+	/*
+	 * Unlock node before returning result; will be one of:

Hrm... looks like a comment suffering from copy/paste without updating
for the new context.

+	 * -ENOENT : index is for empty phandle
+	 * -EINVAL : parsing error on data
+	 * [1..n]  : Number of phandle (count mode; when index = -1)
+	 */
+	rc = index < 0 ? cur_index : -ENOENT;
+ err:
+	return rc;
+}
+
+struct node *propval_parse_phandle(struct node *root,
+				     struct property *prop, int index)
+{
+	struct of_phandle_args args;
+
+	if (index < 0)
+		return NULL;

Again, this is a bug in the caller; should be an assert().

This change needs a justification.

 	p = strchr(path, '/');
 
 	for_each_child(tree, child) {

-- 
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140309/089cd3f7/attachment-0001.sig>