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

[Sean Anderson]

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 at 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 at seco.com>
>> 
>> --Sean
> 
> 
> Can we get this committed?

+CC Tom