[Pdbg] [PATCH 07/23] libpdbg: Create virtual nodes based on device-tree-path property

[Alistair Popple]

On Thursday, 19 September 2019 12:33:17 PM AEST Amitay Isaacs wrote:
> To create a virtual node, "device-tree-path" property is used to denote
> the attachment point in the system device tree view.

Can I bike shed the name? :-)

How about "system-path"?

<snip>

> +static void pdbg_targets_init_virtual(struct pdbg_target *node, struct 
pdbg_target *root)
> +{
> +	struct pdbg_target *vnode, *parent, *child = NULL;
> +	struct dt_property *prop;
> +	const char *dtree_path;
> +	char *parent_path, *sep;
> +	size_t len;
> +
> +	dtree_path = (const char *)pdbg_target_property(node, "device-tree-path", 
&len);
> +	if (!dtree_path)
> +		goto skip;
> +
> +	vnode = dt_find_by_path(root, dtree_path);
> +	if (vnode) {
> +		if (vnode->vnode)
> +			goto skip;
> +		else
> +			goto link_vnode;
> +	}
> +
> +	parent_path = strdup(dtree_path);
> +	assert(parent_path);
> +
> +	sep = strrchr(parent_path, '/');
> +	if (!sep || sep[1] == '\0') {
> +		char *path = dt_get_path(node);
> +		PR_ERROR("Invalid path reference \"%s\" for node %s\n", dtree_path, 
path);
> +		free(path);
> +		free(parent_path);
> +		goto skip;
> +	}
> +
> +	*sep = '\0';
> +
> +	parent = dt_find_by_path(root, parent_path);
> +	if (!parent) {
> +		char *path = dt_get_path(node);
> +		PR_ERROR("Invalid path reference \"%s\" for node %s\n", dtree_path, 
path);
> +		free(path);
> +		free(parent_path);
> +		goto skip;
> +	}
> +
> +	vnode = dt_new_node(sep+1, NULL, 0);
> +	assert(vnode);
> +
> +	free(parent_path);
> +
> +	if (!dt_attach_node(parent, vnode))
> +		goto skip;
> +
> +	PR_DEBUG("Created virtual node %s\n", dtree_path);
> +
> +link_vnode:
> +	node->vnode = vnode;
> +	vnode->vnode = node;
> +
> +	while ((prop = list_pop(&vnode->properties, struct dt_property, list)))
> +		list_add_tail(&node->properties, &prop->list);

This isn't terrible to follow, but I wonder if it couldn't also be refactored 
to be a little cleaner by splitting link_vnode and vnode creation (or perhaps 
just path validation/lookup) into separate functions.

- Alistair

> +skip:
> +	list_for_each(&node->children, child, list) {
> +		if (child->vnode && !child->compatible)
> +			continue;
> +
> +		pdbg_targets_init_virtual(child, root);
> +	}
> +}
> +
>  void pdbg_targets_init(void *fdt)
>  {
>  	/* Root node needs to be valid when this function returns */
> @@ -663,6 +732,7 @@ void pdbg_targets_init(void *fdt)
>  	}
>  
>  	dt_expand(pdbg_dt_root, fdt);
> +	pdbg_targets_init_virtual(pdbg_dt_root, pdbg_dt_root);
>  }
>  
>  char *pdbg_target_path(struct pdbg_target *target)
>