From: Eduardo Habkost <ehabkost@redhat.com>
To: Ziqiao Kong <ziqiaokong@gmail.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
Richard Henderson <richard.henderson@linaro.org>,
QEMU Developers <qemu-devel@nongnu.org>
Subject: Re: [PATCH v4 1/2] target/i386: Trivial code motion
Date: Thu, 27 May 2021 17:10:08 -0400 [thread overview]
Message-ID: <20210527211008.axpaxs4pasy57rfv@habkost.net> (raw)
In-Reply-To: <CAM0BWNAOAZucbfj3KnWS5_QQeOQUQoWkU9ZSG1xPFVH9BK2=wA@mail.gmail.com>
On Tue, May 18, 2021 at 10:53:51AM +0800, Ziqiao Kong wrote:
> On Tue, May 18, 2021 at 4:16 AM Eduardo Habkost <ehabkost@redhat.com> wrote:
> >
> > On Fri, May 07, 2021 at 04:00:56PM +0800, Ziqiao Kong wrote:
> > > Move the float translation case to a new block by a new pair of braces.
> >
> > If you are just adding braces around the code, do you really need
> > to reindent all the code? I don't see any mention of `switch`
> > statements on style.rst, but I see 235 existing cases where the
> > brackets are aligned below the `c` in `case`.
>
> The indention style is from the translate.c itself like here:
>
> https://github.com/qemu/qemu/blob/master/target/i386/tcg/translate.c#L5634
>
> There are also many similar cases in translate.c.
Oh, right. Makes sense to me (and sorry for not noticing this
before).
>
> >
> > In either case, I'm looking for a description of "why", not
> > "what", but I couldn't find it. Why are the braces necessary or
> > useful here?
>
> I have to declare some variables in this case scope, like last_addr, last_seg,
> update_fdp and update_fip according to the previous review. Without the braces
> here, they have to be declared at the beginning of the function like
> the v2 patch.
> Shall I state that in the commit message?
Yes, please. Ideally every commit message should state the
reason for the change.
--
Eduardo
prev parent reply other threads:[~2021-05-27 21:11 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-07 8:00 [PATCH v4 1/2] target/i386: Trivial code motion Ziqiao Kong
2021-05-07 8:00 ` [PATCH v4 2/2] target/i386: Correct implementation for FCS, FIP, FDS and FDP Ziqiao Kong
2021-05-13 16:44 ` Ziqiao Kong
2021-05-17 20:29 ` Eduardo Habkost
2021-05-18 3:06 ` Ziqiao Kong
2021-05-24 7:41 ` Ziqiao Kong
2021-05-27 21:08 ` Eduardo Habkost
2021-05-17 20:16 ` [PATCH v4 1/2] target/i386: Trivial code motion Eduardo Habkost
2021-05-18 2:53 ` Ziqiao Kong
2021-05-27 21:10 ` Eduardo Habkost [this message]
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=20210527211008.axpaxs4pasy57rfv@habkost.net \
--to=ehabkost@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=richard.henderson@linaro.org \
--cc=ziqiaokong@gmail.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.