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 <[email protected] 
<mailto:[email protected]>> 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 <[email protected] 
<mailto:[email protected]>>
     > Cc: Simon Glass <[email protected] <mailto:[email protected]>>
     > Cc: Sean Anderson <[email protected] <mailto:[email protected]>>
     > Cc: Rasmus Villemoes <[email protected] 
<mailto:[email protected]>>
     > Cc: Farhan Ali <[email protected] <mailto:[email protected]>>
     > Cc: "[email protected] <mailto:[email protected]>" <[email protected] 
<mailto:[email protected]>>
     > Cc: Patrick Delaunay <[email protected] 
<mailto:[email protected]>>
     >
     > ---
     >   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

    --Sean

     >       nextin = 0;
     >   #ifndef __U_BOOT__
     >       pi->pgrp = -1;
     >



Regards,

Farhan

Reply via email to