linux-kernel - Re: using IS_ENABLED(CONFIG

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 for Android: free password hash cracker in your pocket

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

Message-ID: <4200245.e01Ct9ua6M@wuerfel>
Date:	Mon, 16 Nov 2015 09:52:16 +0100
From:	Arnd Bergmann <arnd@...db.de>
To:	Vineet Gupta <Vineet.Gupta1@...opsys.com>
Cc:	Geert Uytterhoeven <geert@...ux-m68k.org>,
	Rasmus Villemoes <linux@...musvillemoes.dk>,
	Paul Gortmaker <paul.gortmaker@...driver.com>,
	Ingo Molnar <mingo@...hat.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	"H. Peter Anvin" <hpa@...or.com>, Michal Marek <mmarek@...e.cz>,
	"linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	arcml <linux-snps-arc@...ts.infradead.org>
Subject: Re: using IS_ENABLED(CONFIG_xyz) effectively

On Monday 16 November 2015 08:35:05 Vineet Gupta wrote:
> Hi Geert,
> 
> On Monday 16 November 2015 01:58 PM, Geert Uytterhoeven wrote:
> > Hi Vineet,
> >
> > On Mon, Nov 16, 2015 at 9:00 AM, Vineet Gupta
> > <Vineet.Gupta1@...opsys.com> wrote:
> >> I've been using IS_ENABLED for some time and once in a while run into an issue
> >> which prevents seamless use. Hence posing this question to experts in the area.
> >>
> >> C macro processor evaluates the ensuing control block even if IS_ENABLED evaluates
> >> to false. This requires dummy #defines or worse still removing usage of IS_ENABLED
> >> altogether.
> >>
> >> e.g. In example below even for ARCOMPACT builds, we need the ARCV2 specific define
> >> ARCV2_IRQ_DEF_PRIO.
> >>
> >> void arch_cpu_idle(void)
> >> {
> >>         if (is_isa_arcompact()) {    <---- IS_ENABLED(CONFIG_ISA_ARCOMPACT)
> >>                 __asm__("sleep 0x3");
> >>         } else {
> >>                 const int arg = 0x10 | ARCV2_IRQ_DEF_PRIO;
> >>                __asm__("sleep 0x10");
> >>         }
> >> }
> >>
> >> One could argue that the interface needs to be cleanly defined to not have such
> >> specific #defines in common code in first place. However sometime that becomes
> >> just too tedious.
> >>
> >> Is there a way to get around by this ?
> > Use #ifdef CONFIG_...?
> >
> > The advantage of IS_ENABLED() over #ifdef is that it allows compile-testing of
> > the disabled code path. Of course it should only be compiled if it makes
> > sense. And that's exactly what you're running into.
> 
> And I thought it was to de-uglify the code with same semantics - which doesn't
> seem to be the case !

I would still try to do this with if(IS_ENABLED()) or another macro
like you have above. The problem you ran into with the macro definitions
is that you have conflicting header files:

#ifdef CONFIG_ISA_ARCOMPACT
#include <asm/irqflags-compact.h>
#else
#include <asm/irqflags-arcv2.h>
#endif

This has other (small) problems too, because you get possibly conflicting
symbols (e.g. arch_local_irq_enable but also the register names) that
make it harder to follow what's going on when reading the code.
If you prefix all the defines in the two headers with the respective name,
you can just include both headers and avoid this:

static inline void arch_local_irq_enable(void)
{
	if (IS_ENABLED(CONFIG_ISA_ARCCOMPACT))
		return arccompact_local_irq_enable();
	else
		return arcv2_local_irq_enable();
}

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/