* [PATCH] fastboot: only look up real partition names when no alias exists
@ 2021-12-16 10:26 Matthias Schiffer
2021-12-17 23:20 ` Sean Anderson
2022-01-28 20:15 ` Tom Rini
0 siblings, 2 replies; 6+ messages in thread
From: Matthias Schiffer @ 2021-12-16 10:26 UTC (permalink / raw)
To: u-boot; +Cc: Matthias Schiffer
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.
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);
}
/**
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] fastboot: only look up real partition names when no alias exists
2021-12-16 10:26 [PATCH] fastboot: only look up real partition names when no alias exists Matthias Schiffer
@ 2021-12-17 23:20 ` Sean Anderson
2022-01-26 9:54 ` Matthias Schiffer
2022-01-28 20:15 ` Tom Rini
1 sibling, 1 reply; 6+ messages in thread
From: Sean Anderson @ 2021-12-17 23:20 UTC (permalink / raw)
To: Matthias Schiffer, u-boot
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fastboot: only look up real partition names when no alias exists
2021-12-17 23:20 ` Sean Anderson
@ 2022-01-26 9:54 ` Matthias Schiffer
2022-01-27 16:22 ` Sean Anderson
0 siblings, 1 reply; 6+ messages in thread
From: Matthias Schiffer @ 2022-01-26 9:54 UTC (permalink / raw)
To: Sean Anderson, u-boot
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?
Kind regards,
Matthias
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fastboot: only look up real partition names when no alias exists
2022-01-26 9:54 ` Matthias Schiffer
@ 2022-01-27 16:22 ` Sean Anderson
2022-01-27 22:10 ` Tom Rini
0 siblings, 1 reply; 6+ messages in thread
From: Sean Anderson @ 2022-01-27 16:22 UTC (permalink / raw)
To: Matthias Schiffer, u-boot, Tom Rini
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fastboot: only look up real partition names when no alias exists
2022-01-27 16:22 ` Sean Anderson
@ 2022-01-27 22:10 ` Tom Rini
0 siblings, 0 replies; 6+ messages in thread
From: Tom Rini @ 2022-01-27 22:10 UTC (permalink / raw)
To: Sean Anderson; +Cc: Matthias Schiffer, u-boot, Lukasz Majewski
[-- 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 --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fastboot: only look up real partition names when no alias exists
2021-12-16 10:26 [PATCH] fastboot: only look up real partition names when no alias exists Matthias Schiffer
2021-12-17 23:20 ` Sean Anderson
@ 2022-01-28 20:15 ` Tom Rini
1 sibling, 0 replies; 6+ messages in thread
From: Tom Rini @ 2022-01-28 20:15 UTC (permalink / raw)
To: Matthias Schiffer; +Cc: u-boot
[-- Attachment #1: Type: text/plain, Size: 743 bytes --]
On Thu, Dec 16, 2021 at 11:26:38AM +0100, 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.
>
> Also change one use of strcpy() to strlcpy().
>
> Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> Reviewed-by: Sean Anderson <sean.anderson@seco.com>
Applied to u-boot/master, thanks!
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-01-28 20:15 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-16 10:26 [PATCH] fastboot: only look up real partition names when no alias exists Matthias Schiffer
2021-12-17 23:20 ` Sean Anderson
2022-01-26 9:54 ` Matthias Schiffer
2022-01-27 16:22 ` Sean Anderson
2022-01-27 22:10 ` Tom Rini
2022-01-28 20:15 ` Tom Rini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.