diff mbox series

swupdate-progress: Fix reboot-mode message parsing

Message ID F105E9FE-C1BD-4EB1-B79E-79F5E5AD48EB@siemens.com
State Accepted
Headers show
Series swupdate-progress: Fix reboot-mode message parsing | expand

Commit Message

Storm, Christian June 5, 2024, 6:35 a.m. UTC 
The incoming msg.info reads, e.g.,
{"0": {"1": { "reboot-mode" : "no-reboot"}}}
and sscanf() also needs to parse/read over the enclosing
{"0": <...>}
part, though discarding it.

While at it, use static buffer allocation as the BSDs don't have %m

Signed-off-by: Christian Storm <christian.storm@siemens.com>
---
 tools/swupdate-progress.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

Comments

Storm, Christian June 20, 2024, 7:08 a.m. UTC | #1 
Hi,

> On 5. Jun 2024, at 08:35, 'Storm, Christian' via swupdate <swupdate@googlegroups.com> wrote:
> 
> The incoming msg.info reads, e.g.,
> {"0": {"1": { "reboot-mode" : "no-reboot"}}}
> and sscanf() also needs to parse/read over the enclosing
> {"0": <...>}
> part, though discarding it.
> 
> While at it, use static buffer allocation as the BSDs don't have %m
> 
> Signed-off-by: Christian Storm <christian.storm@siemens.com>
> [...]

Is there anything I can help with or clarify with this patch?

Kind regards,
  Christian
Stefano Babic June 20, 2024, 7:18 a.m. UTC | #2 
On 20.06.24 09:08, 'Storm, Christian' via swupdate wrote:
> Hi,
>
>> On 5. Jun 2024, at 08:35, 'Storm, Christian' via swupdate <swupdate@googlegroups.com> wrote:
>>
>> The incoming msg.info reads, e.g.,
>> {"0": {"1": { "reboot-mode" : "no-reboot"}}}
>> and sscanf() also needs to parse/read over the enclosing
>> {"0": <...>}
>> part, though discarding it.
>>
>> While at it, use static buffer allocation as the BSDs don't have %m
>>
>> Signed-off-by: Christian Storm <christian.storm@siemens.com>
>> [...]
>
> Is there anything I can help with or clarify with this patch?
>

No sorry,  I overlooked it in last merge. I have picked up and merged,
thanks !

Best regards,
Stefano

> Kind regards,
>    Christian
>
Storm, Christian June 20, 2024, 7:35 a.m. UTC | #3 
>>> On 5. Jun 2024, at 08:35, 'Storm, Christian' via swupdate <swupdate@googlegroups.com> wrote:
>>> 
>>> The incoming msg.info reads, e.g.,
>>> {"0": {"1": { "reboot-mode" : "no-reboot"}}}
>>> and sscanf() also needs to parse/read over the enclosing
>>> {"0": <...>}
>>> part, though discarding it.
>>> 
>>> While at it, use static buffer allocation as the BSDs don't have %m
>>> 
>>> Signed-off-by: Christian Storm <christian.storm@siemens.com>
>>> [...]
>> 
>> Is there anything I can help with or clarify with this patch?
>> 
> 
> No sorry,  I overlooked it in last merge. I have picked up and merged,
> thanks !

No worries, thanks for merging!

With this (last) patch backported, Civil Infrastructure Platform's wfx <> SWUpdate integration is on track again...


Kind regards,
  Christian
Matt Wood July 18, 2024, 5:25 p.m. UTC | #4 
Hi Christian and Stefano,

I have been chasing down why updates with reboot=false were rebooting ever 
since upgrading to 2024.05.2.

>> The incoming msg.info reads, e.g., {"0": {"1": { "reboot-mode" : 
"no-reboot"}}}

But I see something different in stream_interface.c:
swupdate_progress_info(RUN, CAUSE_REBOOT_MODE , "{ \"reboot-mode\" : 
\"no-reboot\"}");

Console confirms this during an update:
INFO : {"1": { "reboot-mode" : "no-reboot"}}

Am I missing something?  If not, I have a patch ready to submit to fix.

Thanks, Matt
On Thursday, June 20, 2024 at 3:35:38 AM UTC-4 Storm, Christian wrote:

> >>> On 5. Jun 2024, at 08:35, 'Storm, Christian' via swupdate <
> swup...@googlegroups.com> wrote:
> >>> 
> >>> The incoming msg.info reads, e.g.,
> >>> {"0": {"1": { "reboot-mode" : "no-reboot"}}}
> >>> and sscanf() also needs to parse/read over the enclosing
> >>> {"0": <...>}
> >>> part, though discarding it.
> >>> 
> >>> While at it, use static buffer allocation as the BSDs don't have %m
> >>> 
> >>> Signed-off-by: Christian Storm <christi...@siemens.com>
> >>> [...]
> >> 
> >> Is there anything I can help with or clarify with this patch?
> >> 
> > 
> > No sorry, I overlooked it in last merge. I have picked up and merged,
> > thanks !
>
> No worries, thanks for merging!
>
> With this (last) patch backported, Civil Infrastructure Platform's wfx <> 
> SWUpdate integration is on track again...
>
>
> Kind regards,
> Christian
>
> -- 
> Dr. Christian Storm
> Siemens AG, Technology, T CED OES-DE
> Otto-Hahn-Ring 6, 81739 Munich, Germany
>
>
Matt Wood July 18, 2024, 5:39 p.m. UTC | #5 
Sorry, I misspoke about stream_interface.c, I see the leading {"%d" : %s} 
in the swupdate_progress_info() definition.  However I don't actually see 
it on the console and sscanf returns 0...will debug some more but any 
thoughts would be appreciated.

Thanks, Matt

On Thursday, July 18, 2024 at 1:25:30 PM UTC-4 Matt Wood wrote:

> Hi Christian and Stefano,
>
> I have been chasing down why updates with reboot=false were rebooting ever 
> since upgrading to 2024.05.2.
>
> >> The incoming msg.info reads, e.g., {"0": {"1": { "reboot-mode" : 
> "no-reboot"}}}
>
> But I see something different in stream_interface.c:
> swupdate_progress_info(RUN, CAUSE_REBOOT_MODE , "{ \"reboot-mode\" : 
> \"no-reboot\"}");
>
> Console confirms this during an update:
> INFO : {"1": { "reboot-mode" : "no-reboot"}}
>
> Am I missing something?  If not, I have a patch ready to submit to fix.
>
> Thanks, Matt
> On Thursday, June 20, 2024 at 3:35:38 AM UTC-4 Storm, Christian wrote:
>
>> >>> On 5. Jun 2024, at 08:35, 'Storm, Christian' via swupdate <
>> swup...@googlegroups.com> wrote: 
>> >>> 
>> >>> The incoming msg.info reads, e.g., 
>> >>> {"0": {"1": { "reboot-mode" : "no-reboot"}}} 
>> >>> and sscanf() also needs to parse/read over the enclosing 
>> >>> {"0": <...>} 
>> >>> part, though discarding it. 
>> >>> 
>> >>> While at it, use static buffer allocation as the BSDs don't have %m 
>> >>> 
>> >>> Signed-off-by: Christian Storm <christi...@siemens.com> 
>> >>> [...] 
>> >> 
>> >> Is there anything I can help with or clarify with this patch? 
>> >> 
>> > 
>> > No sorry, I overlooked it in last merge. I have picked up and merged, 
>> > thanks ! 
>>
>> No worries, thanks for merging! 
>>
>> With this (last) patch backported, Civil Infrastructure Platform's wfx <> 
>> SWUpdate integration is on track again... 
>>
>>
>> Kind regards, 
>> Christian 
>>
>> -- 
>> Dr. Christian Storm 
>> Siemens AG, Technology, T CED OES-DE 
>> Otto-Hahn-Ring 6, 81739 Munich, Germany 
>>
>>
Matt Wood July 18, 2024, 6:45 p.m. UTC | #6 
Sorry again, I've been staring at too many strings.  My first email is 
correct:

>> The incoming msg.info reads, e.g., {"0": {"1": { "reboot-mode" : 
"no-reboot"}}}

This does not seem to be the case.  swupdate_progress_info() creates {"%d": 
{ "reboot-mode" : "no-reboot"}}

Is this supposed to pass the RECOVERY_STATUS as the leading attribute to 
match your comment instead?
i.e:
snprintf(pprog->msg.info, sizeof(pprog->msg.info), "{\"%d\": {\"%d\": 
%s}}", status, cause, info);

Or, if not the first attribute is to be discarded anyway so simply 
reverting the string parsing should be sufficient, no?

diff --git a/tools/swupdate-progress.c b/tools/swupdate-progress.c
index 14040757..5e371627 100644
--- a/tools/swupdate-progress.c
+++ b/tools/swupdate-progress.c
@@ -354,7 +354,7 @@ int main(int argc, char **argv)
                         * will be added, JSON lib should be linked.
                         * NOTE: Until then, the exact string format is 
imperative!
                         */
-                       n = sscanf(msg.info, "{\"%*d\": {\"%d\": { 
\"reboot-mode\" : \"%19[-a-z]\"}}}",
+                       n = sscanf(msg.info, "{\"%d\": { \"reboot-mode\" : 
\"%19[-a-z]\"}}",
                                   &cause, reboot_mode);
                        if (n == 2) {
                                if (cause == CAUSE_REBOOT_MODE) {

Thanks, Matt
On Thursday, July 18, 2024 at 1:39:01 PM UTC-4 Matt Wood wrote:

> Sorry, I misspoke about stream_interface.c, I see the leading {"%d" : %s} 
> in the swupdate_progress_info() definition.  However I don't actually see 
> it on the console and sscanf returns 0...will debug some more but any 
> thoughts would be appreciated.
>
> Thanks, Matt
>
> On Thursday, July 18, 2024 at 1:25:30 PM UTC-4 Matt Wood wrote:
>
>> Hi Christian and Stefano,
>>
>> I have been chasing down why updates with reboot=false were rebooting 
>> ever since upgrading to 2024.05.2.
>>
>> >> The incoming msg.info reads, e.g., {"0": {"1": { "reboot-mode" : 
>> "no-reboot"}}}
>>
>> But I see something different in stream_interface.c:
>> swupdate_progress_info(RUN, CAUSE_REBOOT_MODE , "{ \"reboot-mode\" : 
>> \"no-reboot\"}");
>>
>> Console confirms this during an update:
>> INFO : {"1": { "reboot-mode" : "no-reboot"}}
>>
>> Am I missing something?  If not, I have a patch ready to submit to fix.
>>
>> Thanks, Matt
>> On Thursday, June 20, 2024 at 3:35:38 AM UTC-4 Storm, Christian wrote:
>>
>>> >>> On 5. Jun 2024, at 08:35, 'Storm, Christian' via swupdate <
>>> swup...@googlegroups.com> wrote: 
>>> >>> 
>>> >>> The incoming msg.info reads, e.g., 
>>> >>> {"0": {"1": { "reboot-mode" : "no-reboot"}}} 
>>> >>> and sscanf() also needs to parse/read over the enclosing 
>>> >>> {"0": <...>} 
>>> >>> part, though discarding it. 
>>> >>> 
>>> >>> While at it, use static buffer allocation as the BSDs don't have %m 
>>> >>> 
>>> >>> Signed-off-by: Christian Storm <christi...@siemens.com> 
>>> >>> [...] 
>>> >> 
>>> >> Is there anything I can help with or clarify with this patch? 
>>> >> 
>>> > 
>>> > No sorry, I overlooked it in last merge. I have picked up and merged, 
>>> > thanks ! 
>>>
>>> No worries, thanks for merging! 
>>>
>>> With this (last) patch backported, Civil Infrastructure Platform's wfx 
>>> <> SWUpdate integration is on track again... 
>>>
>>>
>>> Kind regards, 
>>> Christian 
>>>
>>> -- 
>>> Dr. Christian Storm 
>>> Siemens AG, Technology, T CED OES-DE 
>>> Otto-Hahn-Ring 6, 81739 Munich, Germany 
>>>
>>>
Stefano Babic July 18, 2024, 7:48 p.m. UTC | #7 
On 18.07.24 20:45, Matt Wood wrote:
> Sorry again, I've been staring at too many strings.  My first email is
> correct:
>
>  >> The incoming msg.info reads, e.g., {"0": {"1": { "reboot-mode" :
> "no-reboot"}}}
>
> This does not seem to be the case. swupdate_progress_info() creates
> {"%d": { "reboot-mode" : "no-reboot"}}
>
> Is this supposed to pass the RECOVERY_STATUS as the leading attribute to
> match your comment instead?
> i.e:
> snprintf(pprog->msg.info, sizeof(pprog->msg.info), "{\"%d\": {\"%d\":
> %s}}", status, cause, info);
>
> Or, if not the first attribute is to be discarded anyway so simply
> reverting the string parsing should be sufficient, no?
>
> diff --git a/tools/swupdate-progress.c b/tools/swupdate-progress.c
> index 14040757..5e371627 100644
> --- a/tools/swupdate-progress.c
> +++ b/tools/swupdate-progress.c
> @@ -354,7 +354,7 @@ int main(int argc, char **argv)
>                           * will be added, JSON lib should be linked.
>                           * NOTE: Until then, the exact string format is
> imperative!
>                           */
> -                       n = sscanf(msg.info, "{\"%*d\": {\"%d\": {
> \"reboot-mode\" : \"%19[-a-z]\"}}}",
> +                       n = sscanf(msg.info, "{\"%d\": { \"reboot-mode\"
> : \"%19[-a-z]\"}}",
>                                     &cause, reboot_mode);
>                          if (n == 2) {
>                                  if (cause == CAUSE_REBOOT_MODE) {
>

I confirm the issue and it is solved by your patch, so send a formal
patch, thanks.

Best regards,
Stefano

> Thanks, Matt
> On Thursday, July 18, 2024 at 1:39:01 PM UTC-4 Matt Wood wrote:
>
>     Sorry, I misspoke about stream_interface.c, I see the leading {"%d"
>     : %s} in the swupdate_progress_info() definition.  However I don't
>     actually see it on the console and sscanf returns 0...will debug
>     some more but any thoughts would be appreciated.
>
>     Thanks, Matt
>
>     On Thursday, July 18, 2024 at 1:25:30 PM UTC-4 Matt Wood wrote:
>
>         Hi Christian and Stefano,
>
>         I have been chasing down why updates with reboot=false were
>         rebooting ever since upgrading to 2024.05.2.
>
>          >> The incoming msg.info <http://msg.info> reads, e.g., {"0":
>         {"1": { "reboot-mode" : "no-reboot"}}}
>
>         But I see something different in stream_interface.c:
>         swupdate_progress_info(RUN, CAUSE_REBOOT_MODE , "{
>         \"reboot-mode\" : \"no-reboot\"}");
>
>         Console confirms this during an update:
>         INFO : {"1": { "reboot-mode" : "no-reboot"}}
>
>         Am I missing something?  If not, I have a patch ready to submit
>         to fix.
>
>         Thanks, Matt
>         On Thursday, June 20, 2024 at 3:35:38 AM UTC-4 Storm, Christian
>         wrote:
>
>              >>> On 5. Jun 2024, at 08:35, 'Storm, Christian' via
>             swupdate <swup...@googlegroups.com> wrote:
>              >>>
>              >>> The incoming msg.info <http://msg.info> reads, e.g.,
>              >>> {"0": {"1": { "reboot-mode" : "no-reboot"}}}
>              >>> and sscanf() also needs to parse/read over the enclosing
>              >>> {"0": <...>}
>              >>> part, though discarding it.
>              >>>
>              >>> While at it, use static buffer allocation as the BSDs
>             don't have %m
>              >>>
>              >>> Signed-off-by: Christian Storm <christi...@siemens.com>
>              >>> [...]
>              >>
>              >> Is there anything I can help with or clarify with this
>             patch?
>              >>
>              >
>              > No sorry, I overlooked it in last merge. I have picked up
>             and merged,
>              > thanks !
>
>             No worries, thanks for merging!
>
>             With this (last) patch backported, Civil Infrastructure
>             Platform's wfx <> SWUpdate integration is on track again...
>
>
>             Kind regards,
>             Christian
>
>             --
>             Dr. Christian Storm
>             Siemens AG, Technology, T CED OES-DE
>             Otto-Hahn-Ring 6, 81739 Munich, Germany
>
> --
> 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/bc446004-eaf2-412c-bf51-838bdf758a42n%40googlegroups.com <https://groups.google.com/d/msgid/swupdate/bc446004-eaf2-412c-bf51-838bdf758a42n%40googlegroups.com?utm_medium=email&utm_source=footer>.
diff mbox series

Patch

diff --git a/tools/swupdate-progress.c b/tools/swupdate-progress.c
index 584910d4..14040757 100644
--- a/tools/swupdate-progress.c
+++ b/tools/swupdate-progress.c
@@ -339,7 +339,7 @@  int main(int argc, char **argv)
 		 * Be sure that string in message are Null terminated
 		 */
 		if (msg.infolen > 0) {
-			char *reboot_mode;
+			char reboot_mode[20] = { 0 };
 			int n, cause;
 
 			if (msg.infolen >= sizeof(msg.info) - 1) {
@@ -354,15 +354,14 @@  int main(int argc, char **argv)
 			 * will be added, JSON lib should be linked.
 			 * NOTE: Until then, the exact string format is imperative!
 			 */
-			n = sscanf(msg.info, "{\"%d\": { \"reboot-mode\" : \"%m[-a-z]\"}}",
-				   &cause, &reboot_mode);
+			n = sscanf(msg.info, "{\"%*d\": {\"%d\": { \"reboot-mode\" : \"%19[-a-z]\"}}}",
+				   &cause, reboot_mode);
 			if (n == 2) {
 				if (cause == CAUSE_REBOOT_MODE) {
 					if (!strcmp(reboot_mode, "no-reboot")) {
 						disable_reboot = true;
 					}
 				}
-				free(reboot_mode);
 			}
 		}
 		msg.cur_image[sizeof(msg.cur_image) - 1] = '\0';