linux-kernel - Re: [PATCH] genirq: Make - vs ?: precedence explicit

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] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.20.1711281442060.1723@nanos>
Date:   Tue, 28 Nov 2017 14:43:51 +0100 (CET)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Rasmus Villemoes <rasmus.villemoes@...vas.dk>
cc:     Kees Cook <keescook@...omium.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] genirq: Make - vs ?: precedence explicit

On Tue, 28 Nov 2017, Rasmus Villemoes wrote:
> On 2017-11-22 21:56, Kees Cook wrote:
> > Noticed with a Clang build. This improves the readability of the ?:
> > expression, as it has lower precedence than the - expression. Show
> > explicitly that - is evaluated first.
> > 
> > Cc: Thomas Gleixner <tglx@...utronix.de>
> > Signed-off-by: Kees Cook <keescook@...omium.org>
> > ---
> >  kernel/irq/matrix.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/kernel/irq/matrix.c b/kernel/irq/matrix.c
> > index a3cbbc8191c5..7df2480005f8 100644
> > --- a/kernel/irq/matrix.c
> > +++ b/kernel/irq/matrix.c
> > @@ -384,7 +384,7 @@ unsigned int irq_matrix_available(struct irq_matrix *m, bool cpudown)
> >  {
> >  	struct cpumap *cm = this_cpu_ptr(m->maps);
> >  
> > -	return m->global_available - cpudown ? cm->available : 0;
> > +	return (m->global_available - cpudown) ? cm->available : 0;
> >  }
> 
> I see that this got applied, and that doesn't change the semantics of
> the code. But surely the code is and was buggy, right? From the kernel
> doc, I'm pretty sure the idea is to subtract cm->available if cpudown is
> true, otherwise subtract 0, i.e.
> 
>   return m->global_available - (cpudown ? cm->available : 0);

Yes, you are right. I completely misread it when I merged that patch. Good
catch!

Thanks,

	tglx