[U-Boot] [PATCH 3/5] fdt: Allow stacked overlays phandle references

[Marek Vasut]

On 06/30/2017 06:23 PM, Pantelis Antoniou wrote:

[...]

> +static int overlay_symbol_update(void *fdt, void *fdto)
> +{
> +	int root_sym, ov_sym, prop, path_len, fragment, target;
> +	int len, frag_name_len, ret, rel_path_len;
> +	const char *s;
> +	const char *path;
> +	const char *name;
> +	const char *frag_name;
> +	const char *rel_path;
> +	char *buf = NULL;
> +
> +	root_sym = fdt_subnode_offset(fdt, 0, "__symbols__");
> +	ov_sym = fdt_subnode_offset(fdto, 0, "__symbols__");
> +
> +	/* if neither exist we can't update symbols, but that's OK */
> +	if (root_sym < 0 || ov_sym < 0)
> +		return 0;

If you have symbol table in either the DTO or the base DT, but not in
the other one, wouldn't it make sense to use that one symbol table
instead of bailing out ?

> +	buf = malloc(FDT_PATH_MAX);
> +	if (!buf)
> +		return -FDT_ERR_NOSPACE;

Would it make sense to allocate this on stack ?

> +	/* iterate over each overlay symbol */
> +	fdt_for_each_property_offset(prop, fdto, ov_sym) {
> +
> +		path = fdt_getprop_by_offset(fdto, prop, &name, &path_len);
> +		if (!path) {
> +			ret = path_len;
> +			goto out;
> +		}
> +
> +		/* skip autogenerated properties */
> +		if (!strcmp(name, "name"))
> +			continue;
> +
> +		/* format: /<fragment-name>/__overlay__/<relative-subnode-path> */
> +
> +		if (*path != '/') {
> +			ret = -FDT_ERR_BADVALUE;
> +			goto out;
> +		}
> +
> +		/* get frag name first */

"frag name" ? What is this now, Unreal Tournament ? :)

> +		s = strchr(path + 1, '/');
> +		if (!s) {
> +			ret = -FDT_ERR_BADVALUE;
> +			goto out;
> +		}
> +		frag_name = path + 1;
> +		frag_name_len = s - path - 1;
> +
> +		/* verify format */
> +		len = strlen("/__overlay__/");
> +		if (strncmp(s, "/__overlay__/", len)) {
> +			ret = -FDT_ERR_NOTFOUND;
> +			goto out;
> +		}
> +
> +		rel_path = s + len;
> +		rel_path_len = strlen(rel_path);
> +
> +		/* find the fragment index in which the symbol lies */
> +		fdt_for_each_subnode(fragment, fdto, 0) {
> +
> +			s = fdt_get_name(fdto, fragment, &len);
> +			if (!s)
> +				continue;
> +
> +			/* name must match */
> +			if (len == frag_name_len && !memcmp(s, frag_name, len))
> +				break;
> +		}
> +
> +		/* not found? */
> +		if (fragment < 0) {
> +			ret = -FDT_ERR_NOTFOUND;
> +			goto out;
> +		}
> +
> +		/* an __overlay__ subnode must exist */
> +		ret = fdt_subnode_offset(fdto, fragment, "__overlay__");
> +		if (ret < 0)
> +			goto out;
> +
> +		/* get the target of the fragment */
> +		ret = overlay_get_target(fdt, fdto, fragment);
> +		if (ret < 0)
> +			goto out;
> +		target = ret;
> +
> +		/* get the path of the target */
> +		ret = fdt_get_path(fdt, target, buf, FDT_PATH_MAX);
> +		if (ret < 0)
> +			goto out;
> +
> +		len = strlen(buf);
> +
> +		/* if the target is root strip leading / */
> +		if (len == 1 && buf[0] == '/')
> +			len = 0;
> +
> +		/* make sure we have enough space */
> +		if (len + 1 + rel_path_len + 1 > FDT_PATH_MAX) {
> +			ret = -FDT_ERR_NOSPACE;
> +			goto out;
> +		}
> +
> +		/* create new symbol path in place */
> +		buf[len] = '/';
> +		memcpy(buf + len + 1, rel_path, rel_path_len);
> +		buf[len + 1 + rel_path_len] = '\0';

Can snprintf() help somewhere here instead of the memcpy() ?

> +		ret = fdt_setprop_string(fdt, root_sym, name, buf);
> +		if (ret < 0)
> +			goto out;
> +	}
> +
> +	ret = 0;
> +
> +out:
> +	if (buf)
> +		free(buf);
> +
> +	return ret;
> +}
> +
>  int fdt_overlay_apply(void *fdt, void *fdto)
>  {
>  	uint32_t delta = fdt_get_max_phandle(fdt);
> @@ -654,6 +796,10 @@ int fdt_overlay_apply(void *fdt, void *fdto)
>  	if (ret)
>  		goto err;
>  
> +	ret = overlay_symbol_update(fdt, fdto);
> +	if (ret)
> +		goto err;
> +
>  	/*
>  	 * The overlay has been damaged, erase its magic.
>  	 */
> 


-- 
Best regards,
Marek Vasut