[PATCH] common: The do_repeat flag interferes with commands issued via run_command API
seanga2 at gmail.com
Sat May 29 17:49:57 CEST 2021
On 5/28/21 2:34 PM, Farhan Ali wrote:
> Hi Sean,
> Thanks for taking a look. I will add the changes you requested.
> However a test case might be difficult to add in the automated tests.
> This problem only happens if there is an asynchronous event ( packet
What do you mean by an "asynchronous event"?
> received/Button pressed ) which triggers a run_command API call AND
> when the user has entered some console commands which have enabled the
> do_repeat flag. If the do_repeat flag is set AND the command issued
> via the run_command API is a 'non-repeatable' command (e.g mmc write),
> the command is ignored.
Ok, so if I understand correctly, this occurs when someone uses a
repeatable command which also calls run_command. Perhaps you can trigger
this behavior with a python test?
> On Thu, May 27, 2021 at 4:24 PM Sean Anderson <seanga2 at gmail.com <mailto:seanga2 at gmail.com>> wrote:
> > Re: [PATCH] common: The do_repeat flag interferes with commands issued via run_command API
> The tag here should be "hush: ..." The subject should be an action,
> such as "Clear do_repeat flag after running commands".
> On 5/27/21 5:24 PM, Farhan Ali wrote:
> > Must clear the do_repeat flag once it is consumed.
> What is "it" here? Please add a few more sentences describing why you
> want to change this. For example, you could note that do_repeat is a
> file-level variable which is used by get_user_input to signal when a
> command should be repeated. Though, I wonder why we don't set the flags
> in the first place...
> 'it' refers to the do_repeat flag.
I found this wording confusing because the flag itself is not really
"consumed", per se. Perhaps "used" is a better verb?
> > Signed-off-by: Farhan Ali <farhan.ali at broadcom.com <mailto:farhan.ali at broadcom.com>>
> > Cc: Simon Glass <sjg at chromium.org <mailto:sjg at chromium.org>>
> > Cc: Sean Anderson <seanga2 at gmail.com <mailto:seanga2 at gmail.com>>
> > Cc: Rasmus Villemoes <rasmus.villemoes at prevas.dk <mailto:rasmus.villemoes at prevas.dk>>
> > Cc: Farhan Ali <farhan.ali at broadcom.com <mailto:farhan.ali at broadcom.com>>
> > Cc: "peng.wang at smartm.com <mailto:peng.wang at smartm.com>" <peng.wang at smartm.com <mailto:peng.wang at smartm.com>>
> > Cc: Patrick Delaunay <patrick.delaunay at foss.st.com <mailto:patrick.delaunay at foss.st.com>>
> > ---
> > common/cli_hush.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> > diff --git a/common/cli_hush.c b/common/cli_hush.c
> > index 1467ff81b3..1c9adf5683 100644
> > --- a/common/cli_hush.c
> > +++ b/common/cli_hush.c
> > @@ -1559,6 +1559,11 @@ static int run_pipe_real(struct pipe *pi)
> > # endif
> > #endif /* __U_BOOT__ */
> > + /* Clear do_repeat after consumption, avoids conflicts
> Multi-line comments should start with a blank line.
> > + * with cmds issued via run_command API
> > + */
> > + do_repeat = 0;
> > +
> Can you add a test case for this?
> So a use case could be as follows:
> (1) User issues several repeated commands on console. This sets do_repeat flag internally
> (2) Webserver embedded in main polling loop of uboot detects an image upgrade request
> (3) Image downloads, about to be written via run_command("mmc write etc.")
> (4) Since 'mmc write' is a non-repeatable command, issuing it with do_repeat flag set results in command getting ignored
> (5) Image upgrade fails
> > nextin = 0;
> > #ifndef __U_BOOT__
> > pi->pgrp = -1;
More information about the U-Boot