diff mbox series

x86: mtrr: Fix parsing of "mtrr list" command

Message ID 20200814075524.554494-1-wolfgang.wallner@br-automation.com
State Accepted
Commit cf9f38064d048ac12cd3a7e1a40a84aa8071f712
Delegated to: Bin Meng
Headers show
Series x86: mtrr: Fix parsing of "mtrr list" command | expand

Commit Message

Wolfgang Wallner Aug. 14, 2020, 7:55 a.m. UTC
The command 'mtrr' does not recognize the 'list' subcommand any more
since the code restructuring in commit b2a76b3fe75a ("x86: mtrr:
Restructure so command execution is in one place").

The if-else parsing the command arguments does not take 'list' into
account: the if-branch is intended for no subcommands, the else-branch
is intended for the non-list subcommands (which all expect additional
arguments). Calling the 'mtrr list' subcommand leads to a "return
CMD_RET_USAGE" in the else-branch.

Fix this by changing the else-branch to explicitly checking for
if (cmd != 'l').

Fixes: b2a76b3fe75a ("x86: mtrr: Restructure so command execution is in one place")

Signed-off-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com>

---

 cmd/x86/mtrr.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Simon Glass Aug. 16, 2020, 3:39 a.m. UTC | #1
On Fri, 14 Aug 2020 at 01:55, Wolfgang Wallner
<wolfgang.wallner@br-automation.com> wrote:
>
> The command 'mtrr' does not recognize the 'list' subcommand any more
> since the code restructuring in commit b2a76b3fe75a ("x86: mtrr:
> Restructure so command execution is in one place").
>
> The if-else parsing the command arguments does not take 'list' into
> account: the if-branch is intended for no subcommands, the else-branch
> is intended for the non-list subcommands (which all expect additional
> arguments). Calling the 'mtrr list' subcommand leads to a "return
> CMD_RET_USAGE" in the else-branch.
>
> Fix this by changing the else-branch to explicitly checking for
> if (cmd != 'l').
>
> Fixes: b2a76b3fe75a ("x86: mtrr: Restructure so command execution is in one place")
>
> Signed-off-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
>
> ---
>
>  cmd/x86/mtrr.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

Ah yes, I never actually use it - since it is implicit.

Regards,
Simon
Bin Meng Sept. 1, 2020, 5:23 a.m. UTC | #2
On Fri, Aug 14, 2020 at 3:55 PM Wolfgang Wallner
<wolfgang.wallner@br-automation.com> wrote:
>
> The command 'mtrr' does not recognize the 'list' subcommand any more
> since the code restructuring in commit b2a76b3fe75a ("x86: mtrr:
> Restructure so command execution is in one place").
>
> The if-else parsing the command arguments does not take 'list' into
> account: the if-branch is intended for no subcommands, the else-branch
> is intended for the non-list subcommands (which all expect additional
> arguments). Calling the 'mtrr list' subcommand leads to a "return
> CMD_RET_USAGE" in the else-branch.
>
> Fix this by changing the else-branch to explicitly checking for
> if (cmd != 'l').
>
> Fixes: b2a76b3fe75a ("x86: mtrr: Restructure so command execution is in one place")
>
> Signed-off-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
>
> ---
>
>  cmd/x86/mtrr.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Tested-by: Bin Meng <bmeng.cn@gmail.com>
Bin Meng Sept. 1, 2020, 5:25 a.m. UTC | #3
On Tue, Sep 1, 2020 at 1:23 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Fri, Aug 14, 2020 at 3:55 PM Wolfgang Wallner
> <wolfgang.wallner@br-automation.com> wrote:
> >
> > The command 'mtrr' does not recognize the 'list' subcommand any more
> > since the code restructuring in commit b2a76b3fe75a ("x86: mtrr:
> > Restructure so command execution is in one place").
> >
> > The if-else parsing the command arguments does not take 'list' into
> > account: the if-branch is intended for no subcommands, the else-branch
> > is intended for the non-list subcommands (which all expect additional
> > arguments). Calling the 'mtrr list' subcommand leads to a "return
> > CMD_RET_USAGE" in the else-branch.
> >
> > Fix this by changing the else-branch to explicitly checking for
> > if (cmd != 'l').
> >
> > Fixes: b2a76b3fe75a ("x86: mtrr: Restructure so command execution is in one place")
> >
> > Signed-off-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
> >
> > ---
> >
> >  cmd/x86/mtrr.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> Tested-by: Bin Meng <bmeng.cn@gmail.com>

applied to u-boot-x86, thanks!
diff mbox series

Patch

diff --git a/cmd/x86/mtrr.c b/cmd/x86/mtrr.c
index e118bba5a2..99efecb9d8 100644
--- a/cmd/x86/mtrr.c
+++ b/cmd/x86/mtrr.c
@@ -121,7 +121,8 @@  static int do_mtrr(struct cmd_tbl *cmdtp, int flag, int argc,
 	if (argc < 1 || !cmd) {
 		cmd = 'l';
 		reg = 0;
-	} else {
+	}
+	if (cmd != 'l') {
 		if (argc < 2)
 			return CMD_RET_USAGE;
 		reg = simple_strtoul(argv[1], NULL, 16);