Re: [PATCH 03/13] ppc440: Add a macro to shorten PCIe controller DCR registration

[Philippe Mathieu-Daudé]

On 4/7/23 11:33, BALATON Zoltan wrote:
> On Tue, 4 Jul 2023, Philippe Mathieu-Daudé wrote:
> > On 4/7/23 00:02, BALATON Zoltan wrote:
> > > It is more readable to wrap the complex call to ppc_dcr_register in a
> > > macro when needed repeatedly.
> > >
> > > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> > > ---
> > >   hw/ppc/ppc440_uc.c | 76 +++++++++++++++++-----------------------------
> > >   1 file changed, 28 insertions(+), 48 deletions(-)
> >
> >
> > > +#define PPC440_PCIE_DCR(s, dcrn) \
> > > +    ppc_dcr_register(&(s)->cpu->env, (s)->dcrn_base + (dcrn), s, \
> >
> > '(s), \'
>
> The parenthesis here would be superfluous as it stands alone in a 
> function parameter between commas so no matter what you substitue here 
> should not have an unwanted side effect (unless it has a comma but 
> that's an error anyway) so maybe this is not needed.

Well I noticed because you used it for the 2 other cases, so I'm
just trying to be consistent here. Besides, not using parenthesis
for macro arguments is a bad practice. Problems happen when others
copy code.

> > > +                     &dcr_read_pcie, &dcr_write_pcie)
> > > +
> > > +
> >
> > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> Thanks for the quick review, I'll post a v2 in a few days to wait a bit 
> if anobody else has any other request.
>
> Regards,
> BALATON Zoltan