Re: [PATCH] fastboot: only look up real partition names when no alias exists

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 3241 bytes --]

On Thu, Jan 27, 2022 at 11:22:12AM -0500, Sean Anderson wrote:
> 
> 
> On 1/26/22 4:54 AM, Matthias Schiffer wrote:
> > On Fri, 2021-12-17 at 18:20 -0500, Sean Anderson wrote:
> >> Hi Matthias,
> >> 
> >> On 12/16/21 5:26 AM, Matthias Schiffer wrote:
> >> > Having U-Boot look up the passed partition name even though an
> >> > alias
> >> > exists is unexpected, leading to warning messages (when the alias
> >> > name
> >> > doesn't exist as a real partition name) or the use of the wrong
> >> > partition.
> >> > 
> >> > Change part_get_info_by_name_or_alias() to consider real partitions
> >> > names only if no alias of the same name exists, allowing to use
> >> > aliases
> >> > to override the configuration for existing partition names.
> >> 
> >> Much saner IMO.
> >> 
> >> I think the correct move in the long term is to add a "quiet"
> >> parameter to do_get_part_info (and all its helpers). This is OK as an
> >> incremental improvement.
> >> 
> >> > Also change one use of strcpy() to strlcpy().
> >> > 
> >> > Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com
> >> > >
> >> > ---
> >> >   drivers/fastboot/fb_mmc.c | 29 ++++++++++++-----------------
> >> >   1 file changed, 12 insertions(+), 17 deletions(-)
> >> > 
> >> > diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c
> >> > index 2738dc836e..fb7791d9da 100644
> >> > --- a/drivers/fastboot/fb_mmc.c
> >> > +++ b/drivers/fastboot/fb_mmc.c
> >> > @@ -104,23 +104,18 @@ static int
> >> > part_get_info_by_name_or_alias(struct blk_desc **dev_desc,
> >> >   					  const char *name,
> >> >   					  struct disk_partition *info)
> >> >   {
> >> > -	int ret;
> >> > -
> >> > -	ret = do_get_part_info(dev_desc, name, info);
> >> > -	if (ret < 0) {
> >> > -		/* strlen("fastboot_partition_alias_") + PART_NAME_LEN
> >> > + 1 */
> >> > -		char env_alias_name[25 + PART_NAME_LEN + 1];
> >> > -		char *aliased_part_name;
> >> > -
> >> > -		/* check for alias */
> >> > -		strcpy(env_alias_name, "fastboot_partition_alias_");
> >> > -		strlcat(env_alias_name, name, sizeof(env_alias_name));
> >> > -		aliased_part_name = env_get(env_alias_name);
> >> > -		if (aliased_part_name != NULL)
> >> > -			ret = do_get_part_info(dev_desc,
> >> > aliased_part_name,
> >> > -					       info);
> >> > -	}
> >> > -	return ret;
> >> > +	/* strlen("fastboot_partition_alias_") + PART_NAME_LEN + 1 */
> >> > +	char env_alias_name[25 + PART_NAME_LEN + 1];
> >> > +	char *aliased_part_name;
> >> > +
> >> > +	/* check for alias */
> >> > +	strlcpy(env_alias_name, "fastboot_partition_alias_",
> >> > sizeof(env_alias_name));
> >> > +	strlcat(env_alias_name, name, sizeof(env_alias_name));
> >> > +	aliased_part_name = env_get(env_alias_name);
> >> > +	if (aliased_part_name)
> >> > +		name = aliased_part_name;
> >> > +
> >> > +	return do_get_part_info(dev_desc, name, info);
> >> >   }
> >> > 
> >> >   /**
> >> > 
> >> 
> >> Reviewed-by: Sean Anderson <sean.anderson@seco.com>
> >> 
> >> --Sean
> > 
> > 
> > Can we get this committed?
> 
> +CC Tom

There's a number of fastboot things and I guess I need to see what I can
try and review and grab or reassign.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]