Re: [PATCH v4 1/2] target/i386: Trivial code motion

[Eduardo Habkost]

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