Re: git: 7bcc7a0b88cc - stable/14 - libkern: avoid local var in order_base_2()

bz@freebsd.org presented the problem that this change fixed.  I think it 
was in a driver.  It may have been a case in which the macro argument 
had side effects, and having it appear twice in the macro made those 
side effects happen twice.  But I have not preserved the original report 
from bz.  I regret that I did not adequately describe the problem being 
fixed in the commit message.  Perhaps bz can shed some light on the matter.

Doug

On 2/10/25 21:30, Zhenlei Huang wrote:
>
>> On Feb 10, 2025, at 7:15 PM, Doug Moore <dougm@FreeBSD.org> wrote:
>>
>> The branch stable/14 has been updated by dougm:
>>
>> URL: https://cgit.FreeBSD.org/src/commit/?id=7bcc7a0b88ccb5e1fe31de88ab9a17e46859318b
>>
>> commit 7bcc7a0b88ccb5e1fe31de88ab9a17e46859318b
>> Author:     Doug Moore <dougm@FreeBSD.org>
>> AuthorDate: 2024-09-27 23:43:07 +0000
>> Commit:     Doug Moore <dougm@FreeBSD.org>
>> CommitDate: 2025-02-10 10:30:05 +0000
>>
>>     libkern: avoid local var in order_base_2()
>>
>>     order_base_2(n) is implemented with a variable, which keeps it from
>>     being used at file scope. Implement it instead as ilog2(2*n-1), which
>>     produces a different result when 2*n overflows, which appears unlikely
>>     in practice.
>>
>>     Reviewed by:    bz
>>     Differential Revision:  https://reviews.freebsd.org/D46826
>>
>>     (cherry picked from commit b7cbf741d55468ba34305a14ac3acc1c286af034)
>> ---
>> sys/sys/libkern.h | 5 +----
>> 1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/sys/sys/libkern.h b/sys/sys/libkern.h
>> index a10289d72dc7..835e5ffaf0b7 100644
>> --- a/sys/sys/libkern.h
>> +++ b/sys/sys/libkern.h
>> @@ -229,10 +229,7 @@ ilog2_long_long(long long n)
>>
>> #define	ilog2(n) (__builtin_constant_p(n) ? ilog2_const(n) : ilog2_var(n))
>> #define	rounddown_pow_of_two(n)	((__typeof(n))1 << ilog2(n))
>> -#define	order_base_2(n) ({			\
>> -	__typeof(n) _n = (n);			\
> This local var `_n` is within the scope of the macro `order_base_2`, it is surrounded with
> {} and is harmless, so it will not pollute the file scoped variables.
>
> Am I reading the commit message wrong ?
>
> Best regards,
> Zhenlei
>
>> -	_n == 1 ? 0 : 1 + ilog2(_n - 1);	\
>> -})
>> +#define order_base_2(n) ilog2(2*(n)-1)
>> #define	roundup_pow_of_two(n)	((__typeof(n))1 << order_base_2(n))
>>
>> #define	bitcount64(x)	__bitcount64((uint64_t)(x))
>
>