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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250108130254.7704-K-hca@linux.ibm.com>
Date: Wed, 8 Jan 2025 14:02:54 +0100
From: Heiko Carstens <hca@...ux.ibm.com>
To: Akihiko Odaki <akihiko.odaki@...nix.com>
Cc: Dave Martin <Dave.Martin@....com>, Vasily Gorbik <gor@...ux.ibm.com>,
        Alexander Gordeev <agordeev@...ux.ibm.com>,
        Christian Borntraeger <borntraeger@...ux.ibm.com>,
        Sven Schnelle <svens@...ux.ibm.com>,
        Eric Biederman <ebiederm@...ssion.com>, Kees Cook <kees@...nel.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Mark Brown <broonie@...nel.org>, Baoquan He <bhe@...hat.com>,
        Vivek Goyal <vgoyal@...hat.com>, Dave Young <dyoung@...hat.com>,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        linuxppc-dev@...ts.ozlabs.org, linux-s390@...r.kernel.org,
        kexec@...ts.infradead.org, binutils@...rceware.org, devel@...nix.com
Subject: Re: [PATCH v3 5/6] s390/crash: Use note name macros

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@...ux.ibm.com>