Re: [Qemu-devel] [PATCH v3 1/1] configure: Define target access alignment in configure

[Peter Maydell]

On Tue, 30 Jul 2019 at 21:00, Aleksandar Markovic
<address@hidden> wrote:
>
> On Thu, Jul 25, 2019 at 3:25 AM <address@hidden> wrote:
>
> > Rename ALIGNED_ONLY to TARGET_ALIGNED_ONLY for clarity and move
> > defines out of target/foo/cpu.h into configure, as we do with
> > TARGET_WORDS_BIGENDIAN, so that it is always defined early.
> >
> > Also, poison the symbol in include/exec/poison.h to prevent use in
> > common code.
> >
> >
> Hi, Tony.
>
> Thanks for this new version.
>
> When one mentions "also" in the commit message, this is a kind of strong
> indication that the patch should be split into two patches.
>
> So, could you please consider moving "poison" part into a separate patch?

I think in this case the issue is more in the phrasing of the commit
message rather than the patch composition. The patch is
introducing TARGET_ALIGNED_ONLY as something defined
by configure, and the correct status for that macro is "needs to
be in poison.h"; having an intermediate state where the macro
exists but isn't poisoned isn't really a logical split of the work.
(The previous ALIGNED_ONLY didn't have so much need to be
poisoned because it was defined in cpu.h and so the only way to
expect to get it was by pulling in cpu.h.)

thanks
-- PMM