Date: Thu, 14 Oct 2010 09:37:51 -0400 From: John Baldwin <jhb@freebsd.org> To: freebsd-hackers@freebsd.org Cc: Andriy Gapon <avg@icyb.net.ua>, Garrett Cooper <gcooper@freebsd.org> Subject: Re: [PATCH] Bug with powerof2 macro in sys/param.h Message-ID: <201010140937.51953.jhb@freebsd.org> In-Reply-To: <4CB6F068.1070406@icyb.net.ua> References: <AANLkTi=0PSp2KpFTAHTAoosRYdcewiVAxweH4=ivYTLt@mail.gmail.com> <4CB6F068.1070406@icyb.net.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thursday, October 14, 2010 7:58:32 am Andriy Gapon wrote: > on 14/10/2010 00:30 Garrett Cooper said the following: > > I was talking to someone today about this macro, and he noted that > > the algorithm is incorrect -- it fails the base case with ((x) == 0 -- > > which makes sense because 2^(x) cannot equal 0 (mathematically > > impossible, unless you consider the limit as x goes to negative > > infinity as log (0) / log(2) is undefined). I tested out his claim and > > he was right: > > That's kind of obvious given the code. > I think that this might be an intentional optimization. > I guess that it doesn't really make sense to apply powerof2 to zero and the users > of the macro should do the check on their own if they expect zero as input (many > places in the do not allow that). I agree, the current macro is this way on purpose (and straight out of "Hacker's Delight"). Of the existing calls you weren't sure of: sys/dev/cxgb/cxgb_sge.c: while (!powerof2(fl_q_size)) sys/dev/cxgb/cxgb_sge.c: while (!powerof2(jumbo_q_size)) These are fine, will not be zero. sys/x86/x86/local_apic.c: KASSERT(powerof2(count), ("bad count")); sys/x86/x86/local_apic.c: KASSERT(powerof2(align), ("bad align")); These are fine. No code allocates zero IDT vectors. We never allocate IDT vectors for unallocated MSI or MSI-X IRQs. sys/net/flowtable.c: ft->ft_lock_count = 2*(powerof2(mp_maxid + 1) ? (mp_maxid + 1): Clearly, 'mp_maxid + 1' will not be zero (barring a bizarre overflow case which will not happen until we support 2^32 CPUs), so this is fine. sys/i386/pci/pci_pir.c: if (error && !powerof2(pci_link->pl_irqmask)) { This fine. Earlier in the function if pl_irqmask is zero, then all of the pci_pir_choose_irq() calls will fail, so this is only invoked if pl_irqmask is non-zero. In practice pl_irqmask is never zero anyway. I suspect the GEOM ones are also generally safe. -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201010140937.51953.jhb>