Re: [PATCH v3 5/6] s390/crash: Use note name macros

[Heiko Carstens <hca@linux.ibm.com>]

On Wed, Jan 08, 2025 at 01:53:51PM +0900, Akihiko Odaki wrote:
> On 2025/01/08 1:17, Dave Martin wrote:
> > > +#define NT_INIT(buf, type, desc) \
> > > +	(nt_init_name((buf), NT_ ## type, &(desc), sizeof(desc), NN_ ## type))
> > 
> > Nit: this macro name clashes with the naming scheme in elf.h.
> > 
> > I think that there is a (weak) convention that macros with upper-case
> > names don't expand to a C function call; thus, a macro with an upper-
> > case name can be invoked in places where a C function call would not be
> > allowed.  (This convention is not followed everywhere, though -- it's
> > up to the maintainer what they prefer here.)
> 
> I wanted to clarify it is a macro as it concatenates tokens with ##, but I
> also find there are many macros that are named lower-case and performs token
> concatenation.
> 
> S390 maintainers, please tell usr your opinion.

Just make the new macros lower case to avoid the naming scheme
clashes, please. Otherwise it doesn't matter too much.

> > > +#define NT_SIZE(type, desc) (nt_size_name(sizeof(desc), NN_ ## type))
> > 
> > Nit: name prefix clash (again); possibly redundant parentheses.

Same here.

> > > -	size =	nt_size(NT_PRSTATUS, sizeof(struct elf_prstatus));
> > > -	size +=  nt_size(NT_PRFPREG, sizeof(elf_fpregset_t));
> > > -	size +=  nt_size(NT_S390_TIMER, sizeof(sa->timer));
> > > -	size +=  nt_size(NT_S390_TODCMP, sizeof(sa->todcmp));
> > > -	size +=  nt_size(NT_S390_TODPREG, sizeof(sa->todpreg));
> > > -	size +=  nt_size(NT_S390_CTRS, sizeof(sa->ctrs));
> > > -	size +=  nt_size(NT_S390_PREFIX, sizeof(sa->prefix));
> > > +	size =	NT_SIZE(PRSTATUS, struct elf_prstatus);
> > > +	size +=  NT_SIZE(PRFPREG, elf_fpregset_t);
> > > +	size +=  NT_SIZE(S390_TIMER, sa->timer);
> > > +	size +=  NT_SIZE(S390_TODCMP, sa->todcmp);
> > > +	size +=  NT_SIZE(S390_TODPREG, sa->todpreg);
> > > +	size +=  NT_SIZE(S390_CTRS, sa->ctrs);
> > > +	size +=  NT_SIZE(S390_PREFIX, sa->prefix);
> > 
> > It might be worth fixing the funny spacing on these lines, since all
> > the affected lines are being replaced.

Yes, please!

Besides that this looks good:
Acked-by: Heiko Carstens <hca@linux.ibm.com>