qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Dr. David Alan Gilbert" <dave@treblig.org>
To: Phil Dennis-Jordan <lists@philjordan.eu>
Cc: Anastasia Belova <abelova@astralinux.ru>,
	qemu-devel@nongnu.org, sdl.qemu@linuxtesting.org
Subject: Re: [PATCH] monitor: fix cases in switch in memory_dump
Date: Thu, 31 Oct 2024 00:31:09 +0000	[thread overview]
Message-ID: <ZyLPzbDy_0cI7HyD@gallifrey> (raw)
In-Reply-To: <CAGCz3vuqRAkPtRRpqbPM+3z5iqVHOkr5fYotmva=6uqA7JnGcQ@mail.gmail.com>

* Phil Dennis-Jordan (lists@philjordan.eu) wrote:
> On Wed 30. Oct 2024 at 15:09, Anastasia Belova <abelova@astralinux.ru>
> wrote:
> 
> > default case has no condition. So if it is placed
> > higher that other cases, they are unreachable.
> >
> > Move dafult case down.
> >
> 
> The stylistic merits might be debatable, but: the order of cases in a
> switch block in C does not matter, the default case can appear anywhere.
> The other cases are still reachable. So at minimum, the commit message is
> incorrect.

I'd agree;  the analysis is wrong - it works as intended.
As for style, I'd normally agree that 'default' at end makes sense,
but:
  a) I hate duplicating code
  b) in a way this reads nicely:
                 default:
                 case 1:

      'default is the same as case 1'.

Dave

> 
> 
> > Found by Linux Verification Center (linuxtesting.org)
> >
> > Signed-off-by: Anastasia Belova <abelova@astralinux.ru>
> > ---
> >  monitor/hmp-cmds-target.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/monitor/hmp-cmds-target.c b/monitor/hmp-cmds-target.c
> > index ff01cf9d8d..eea8ca047b 100644
> > --- a/monitor/hmp-cmds-target.c
> > +++ b/monitor/hmp-cmds-target.c
> > @@ -189,7 +189,6 @@ static void memory_dump(Monitor *mon, int count, int
> > format, int wsize,
> >          i = 0;
> >          while (i < l) {
> >              switch(wsize) {
> > -            default:
> >              case 1:
> >                  v = ldub_p(buf + i);
> >                  break;
> > @@ -202,6 +201,9 @@ static void memory_dump(Monitor *mon, int count, int
> > format, int wsize,
> >              case 8:
> >                  v = ldq_p(buf + i);
> >                  break;
> > +            default:
> > +                v = ldub_p(buf + i);
> > +                break;
> >              }
> >              monitor_printf(mon, " ");
> >              switch(format) {
> > --
> > 2.47.0
> >
> >
> >
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/


  reply	other threads:[~2024-10-31  0:32 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-30 14:06 [PATCH] monitor: fix cases in switch in memory_dump Anastasia Belova
2024-10-30 19:03 ` Phil Dennis-Jordan
2024-10-31  0:31   ` Dr. David Alan Gilbert [this message]
2024-10-31 10:52     ` Peter Maydell
2024-10-31 11:26       ` Peter Maydell
2024-10-31 11:26   ` Anastasia Belova

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZyLPzbDy_0cI7HyD@gallifrey \
    --to=dave@treblig.org \
    --cc=abelova@astralinux.ru \
    --cc=lists@philjordan.eu \
    --cc=qemu-devel@nongnu.org \
    --cc=sdl.qemu@linuxtesting.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).