[U-Boot] [PATCH] dm: fdtdec: Check full path for alias

[Stephen Warren]

On 04/14/2016 07:02 AM, Michal Simek wrote:
> Fix fdtdec_get_alias_seq() which is not checking full path to certain
> node and it incorrectly provides incorrect seq number.
> Checking full path ensure that if alias is present correct seq number is
> return.
> This problem was found on ZynqMP zcu102 where are several I2C muxes
> where the first bus name is i2c at 0 and for following muxes incorrect seq
> numbers are return.
>
> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
> ---
> I am not sure if there is any macro for max alias length which I can use
> instead of new macro.

> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> index 70acc29c924d..79efcf0cd6b0 100644
> --- a/lib/fdtdec.c
> +++ b/lib/fdtdec.c
> @@ -506,6 +506,8 @@ int fdtdec_add_aliases_for_id(const void *blob, const char *name,
>   	return num_found;
>   }
>
> +#define FDT_ALIAS_PATH_LEN	64
> +
>   int fdtdec_get_alias_seq(const void *blob, const char *base, int offset,
>   			 int *seqp)
>   {
> @@ -514,9 +516,13 @@ int fdtdec_get_alias_seq(const void *blob, const char *base, int offset,
>   	int find_namelen;
>   	int prop_offset;
>   	int aliases;
> +	char buf[FDT_ALIAS_PATH_LEN];
> +
> +	fdt_get_path(blob, offset, buf, sizeof(buf));

Rather the getting the path of the object, and then comparing it against 
the alias, you could check each component separately, and hence not need 
to ever generate the full concatenated path. That would be a lot slower 
though.

Another thought is a variant of fdt_get_path() which also returns the 
required length of the path, so you could do something like:

buf = malloc(DEFAULT_LEN);
ret = fdt_get_path(blob, offset, buf, DEFAULT_LEN, &required_len);
if (ret == -ENOSPC)
     buf = realloc(buf, required_len);
     ret = fdt_get_path(blob, offset, buf, required_len, &required_len);
}
if (ret)
     return ret;

> @@ -533,6 +539,13 @@ int fdtdec_get_alias_seq(const void *blob, const char *base, int offset,

> +		/* Check full path first not to point to incorrect alias */
> +		if (strncmp(prop, buf, min(len, FDT_ALIAS_PATH_LEN)))
> +			continue;
> +
>   		slash = strrchr(prop, '/');
>   		if (strcmp(slash + 1, find_name))
>   			continue;

I would expect the existing check to completely replace the original 
one. IIUC, the comparison is required to be via the full path, and the 
existing code is simply a bug for simplicity.