diff mbox series

[libubootenv] Fix parsing multiple .flags variables

Message ID b93f766a-5a4d-47ae-9c7e-4622b94d857fn@googlegroups.com
State Accepted
Headers show
Series [libubootenv] Fix parsing multiple .flags variables | expand

Commit Message

Bartel Eerdekens Nov. 2, 2021, 1:39 p.m. UTC
From a6d724f7de1aa3d55fe63979848e18d3f8b1562e Mon Sep 17 00:00:00 2001
From: Bartel Eerdekens <bartel.eerdekens@constell8.be>
Date: Tue, 2 Nov 2021 14:36:25 +0100
Subject: [PATCH] Fix parsing multiple .flags variables.

Signed-off-by: Bartel Eerdekens <bartel.eerdekens@constell8.be>
---
 src/uboot_env.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Bartel Eerdekens Nov. 2, 2021, 1:43 p.m. UTC | #1
As noticed when testing multiple approaches using .flags 
(https://groups.google.com/g/swupdate/c/WaAbVixwCQM), only the first entry 
is handled.

As \0 string terminators are added, strlen(flagsvar) is re-evaluated on the 
next while-loop iteration, and ends prematurely, stopping after the 
introduced \0 string terminator.

Op dinsdag 2 november 2021 om 14:39:37 UTC+1 schreef Bartel Eerdekens:

> From a6d724f7de1aa3d55fe63979848e18d3f8b1562e Mon Sep 17 00:00:00 2001
> From: Bartel Eerdekens <bartel.e...@constell8.be>
> Date: Tue, 2 Nov 2021 14:36:25 +0100
> Subject: [PATCH] Fix parsing multiple .flags variables.
>
> Signed-off-by: Bartel Eerdekens <bartel.e...@constell8.be>
> ---
>  src/uboot_env.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/uboot_env.c b/src/uboot_env.c
> index 34962e6..223a300 100644
> --- a/src/uboot_env.c
> +++ b/src/uboot_env.c
> @@ -1077,9 +1077,10 @@ static int libuboot_load(struct uboot_ctx *ctx)
>  #if !defined(NDEBUG)
>   fprintf(stdout, "Environment FLAGS %s\n", flagsvar);
>  #endif
> + unsigned int flagslen = strlen(flagsvar);
>   pvar = flagsvar;
>  
> - while (*pvar && (pvar - flagsvar) < strlen(flagsvar)) {
> + while (*pvar && (pvar - flagsvar) < flagslen) {
>   char *pnext;
>   pval = strchr(pvar, ':');
>   if (!pval)
> -- 
> 2.24.3 (Apple Git-128)
>
>
Stefano Babic Nov. 2, 2021, 1:53 p.m. UTC | #2
Hi Bartel,

On 02.11.21 14:43, Bartel Eerdekens wrote:
> As noticed when testing multiple approaches using .flags 
> (https://groups.google.com/g/swupdate/c/WaAbVixwCQM), only the first 
> entry is handled.
> 
> As \0 string terminators are added, strlen(flagsvar) is re-evaluated on 
> the next while-loop iteration, and ends prematurely, stopping after the 
> introduced \0 string terminator.
> 

This is the right explanation of the bug, and it should be put in the 
commit message instead of letting empty. Can you correct and repost it, 
please ?

Best regards,
Stefano Babic
Bartel Eerdekens Nov. 2, 2021, 3:06 p.m. UTC | #3
From 8ba10e3412924430fdd0a73f42cdda5ec344dcc6 Mon Sep 17 00:00:00 2001
From: Bartel Eerdekens <bartel.eerdekens@constell8.be>
Date: Tue, 2 Nov 2021 14:36:25 +0100
Subject: [PATCH] Fix parsing multiple .flags variables.

strlen(flagsvar) is re-evaluated on each while-loop iteration
As \0 string terminators are added, the loop ends prematurely, stopping 
after the first introduced \0 string terminator.

Signed-off-by: Bartel Eerdekens <bartel.eerdekens@constell8.be>
---
 src/uboot_env.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/uboot_env.c b/src/uboot_env.c
index 34962e6..223a300 100644
--- a/src/uboot_env.c
+++ b/src/uboot_env.c
@@ -1077,9 +1077,10 @@ static int libuboot_load(struct uboot_ctx *ctx)
 #if !defined(NDEBUG)
  fprintf(stdout, "Environment FLAGS %s\n", flagsvar);
 #endif
+ unsigned int flagslen = strlen(flagsvar);
  pvar = flagsvar;
 
- while (*pvar && (pvar - flagsvar) < strlen(flagsvar)) {
+ while (*pvar && (pvar - flagsvar) < flagslen) {
  char *pnext;
  pval = strchr(pvar, ':');
  if (!pval)
Stefano Babic Nov. 2, 2021, 3:14 p.m. UTC | #4
Hi Bartel,

On 02.11.21 16:06, Bartel Eerdekens wrote:
>  From 8ba10e3412924430fdd0a73f42cdda5ec344dcc6 Mon Sep 17 00:00:00 2001
> From: Bartel Eerdekens <bartel.eerdekens@constell8.be>
> Date: Tue, 2 Nov 2021 14:36:25 +0100
> Subject: [PATCH] Fix parsing multiple .flags variables.
> 
> strlen(flagsvar) is re-evaluated on each while-loop iteration
> As \0 string terminators are added, the loop ends prematurely, stopping 
> after the first introduced \0 string terminator.
> 
> Signed-off-by: Bartel Eerdekens <bartel.eerdekens@constell8.be>

I rearranged the message and applied to -master. Please next time use 
"git send-email" to post patches, as your e-mail has made the patch 
itself not appliable - thanks.

Best regards,
Stefano Babic

> ---
>   src/uboot_env.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/uboot_env.c b/src/uboot_env.c
> index 34962e6..223a300 100644
> --- a/src/uboot_env.c
> +++ b/src/uboot_env.c
> @@ -1077,9 +1077,10 @@ static int libuboot_load(struct uboot_ctx *ctx)
>   #if !defined(NDEBUG)
> fprintf(stdout, "Environment FLAGS %s\n", flagsvar);
>   #endif
> +unsigned int flagslen = strlen(flagsvar);
> pvar = flagsvar;
> -while (*pvar && (pvar - flagsvar) < strlen(flagsvar)) {
> +while (*pvar && (pvar - flagsvar) < flagslen) {
> char *pnext;
> pval = strchr(pvar, ':');
> if (!pval)
> -- 
> 2.24.3 (Apple Git-128)
> 
> Op dinsdag 2 november 2021 om 14:53:21 UTC+1 schreef Stefano Babic:
> 
>     Hi Bartel,
> 
>     On 02.11.21 14:43, Bartel Eerdekens wrote:
>      > As noticed when testing multiple approaches using .flags
>      > (https://groups.google.com/g/swupdate/c/WaAbVixwCQM
>     <https://groups.google.com/g/swupdate/c/WaAbVixwCQM>), only the first
>      > entry is handled.
>      >
>      > As \0 string terminators are added, strlen(flagsvar) is
>     re-evaluated on
>      > the next while-loop iteration, and ends prematurely, stopping
>     after the
>      > introduced \0 string terminator.
>      >
> 
>     This is the right explanation of the bug, and it should be put in the
>     commit message instead of letting empty. Can you correct and repost it,
>     please ?
> 
>     Best regards,
>     Stefano Babic
> 
> 
> 
>     -- 
>     =====================================================================
>     DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
>     HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>     Phone: +49-8142-66989-53 <tel:+49%208142%206698953> Fax:
>     +49-8142-66989-80 <tel:+49%208142%206698980> Email: sba...@denx.de
>     =====================================================================
> 
> -- 
> You received this message because you are subscribed to the Google 
> Groups "swupdate" group.
> To unsubscribe from this group and stop receiving emails from it, send 
> an email to swupdate+unsubscribe@googlegroups.com 
> <mailto:swupdate+unsubscribe@googlegroups.com>.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/swupdate/03b326f2-72df-4c29-8460-5246176e996cn%40googlegroups.com 
> <https://groups.google.com/d/msgid/swupdate/03b326f2-72df-4c29-8460-5246176e996cn%40googlegroups.com?utm_medium=email&utm_source=footer>.
Bartel Eerdekens Nov. 3, 2021, 9:07 a.m. UTC | #5
Hi Stefano,

My bad, will do next time!
Thank you for the merge!



Op dinsdag 2 november 2021 om 16:14:31 UTC+1 schreef Stefano Babic:

> Hi Bartel,
>
> On 02.11.21 16:06, Bartel Eerdekens wrote:
> > From 8ba10e3412924430fdd0a73f42cdda5ec344dcc6 Mon Sep 17 00:00:00 2001
> > From: Bartel Eerdekens <bartel.e...@constell8.be>
> > Date: Tue, 2 Nov 2021 14:36:25 +0100
> > Subject: [PATCH] Fix parsing multiple .flags variables.
> > 
> > strlen(flagsvar) is re-evaluated on each while-loop iteration
> > As \0 string terminators are added, the loop ends prematurely, stopping 
> > after the first introduced \0 string terminator.
> > 
> > Signed-off-by: Bartel Eerdekens <bartel.e...@constell8.be>
>
> I rearranged the message and applied to -master. Please next time use 
> "git send-email" to post patches, as your e-mail has made the patch 
> itself not appliable - thanks.
>
> Best regards,
> Stefano Babic
>
> > ---
> >  src/uboot_env.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/uboot_env.c b/src/uboot_env.c
> > index 34962e6..223a300 100644
> > --- a/src/uboot_env.c
> > +++ b/src/uboot_env.c
> > @@ -1077,9 +1077,10 @@ static int libuboot_load(struct uboot_ctx *ctx)
> >  #if !defined(NDEBUG)
> > fprintf(stdout, "Environment FLAGS %s\n", flagsvar);
> >  #endif
> > +unsigned int flagslen = strlen(flagsvar);
> > pvar = flagsvar;
> > -while (*pvar && (pvar - flagsvar) < strlen(flagsvar)) {
> > +while (*pvar && (pvar - flagsvar) < flagslen) {
> > char *pnext;
> > pval = strchr(pvar, ':');
> > if (!pval)
> > -- 
> > 2.24.3 (Apple Git-128)
> > 
> > Op dinsdag 2 november 2021 om 14:53:21 UTC+1 schreef Stefano Babic:
> > 
> > Hi Bartel,
> > 
> > On 02.11.21 14:43, Bartel Eerdekens wrote:
> > > As noticed when testing multiple approaches using .flags
> > > (https://groups.google.com/g/swupdate/c/WaAbVixwCQM
> > <https://groups.google.com/g/swupdate/c/WaAbVixwCQM>), only the first
> > > entry is handled.
> > >
> > > As \0 string terminators are added, strlen(flagsvar) is
> > re-evaluated on
> > > the next while-loop iteration, and ends prematurely, stopping
> > after the
> > > introduced \0 string terminator.
> > >
> > 
> > This is the right explanation of the bug, and it should be put in the
> > commit message instead of letting empty. Can you correct and repost it,
> > please ?
> > 
> > Best regards,
> > Stefano Babic
> > 
> > 
> > 
> > -- 
> > =====================================================================
> > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
> > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> > Phone: +49-8142-66989-53 <+49%208142%206698953> 
> <tel:+49%208142%206698953> Fax:
> > +49-8142-66989-80 <+49%208142%206698980> <tel:+49%208142%206698980> 
> Email: sba...@denx.de
> > =====================================================================
> > 
> > -- 
> > You received this message because you are subscribed to the Google 
> > Groups "swupdate" group.
> > To unsubscribe from this group and stop receiving emails from it, send 
> > an email to swupdate+u...@googlegroups.com 
> > <mailto:swupdate+u...@googlegroups.com>.
> > To view this discussion on the web visit 
> > 
> https://groups.google.com/d/msgid/swupdate/03b326f2-72df-4c29-8460-5246176e996cn%40googlegroups.com 
> > <
> https://groups.google.com/d/msgid/swupdate/03b326f2-72df-4c29-8460-5246176e996cn%40googlegroups.com?utm_medium=email&utm_source=footer
> >.
>
>
> -- 
> =====================================================================
> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-53 <+49%208142%206698953> Fax: +49-8142-66989-80 
> <+49%208142%206698980> Email: sba...@denx.de
> =====================================================================
>
diff mbox series

Patch

diff --git a/src/uboot_env.c b/src/uboot_env.c
index 34962e6..223a300 100644
--- a/src/uboot_env.c
+++ b/src/uboot_env.c
@@ -1077,9 +1077,10 @@  static int libuboot_load(struct uboot_ctx *ctx)
 #if !defined(NDEBUG)
  fprintf(stdout, "Environment FLAGS %s\n", flagsvar);
 #endif
+ unsigned int flagslen = strlen(flagsvar);
  pvar = flagsvar;
 
- while (*pvar && (pvar - flagsvar) < strlen(flagsvar)) {
+ while (*pvar && (pvar - flagsvar) < flagslen) {
  char *pnext;
  pval = strchr(pvar, ':');
  if (!pval)