linux-kernel - Re: [PATCH] powerpc: replace ternary operator with min()

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]

Date:   Wed, 2 Nov 2022 09:23:15 +0000
From:   Christophe Leroy <christophe.leroy@...roup.eu>
To:     Russell Currey <ruscur@...sell.cc>,
        KaiLong Wang <wangkailong@...i.cn>,
        "mpe@...erman.id.au" <mpe@...erman.id.au>,
        "npiggin@...il.com" <npiggin@...il.com>,
        "farosas@...ux.ibm.com" <farosas@...ux.ibm.com>,
        "mirq-linux@...e.qmqm.pl" <mirq-linux@...e.qmqm.pl>,
        "dmitry.osipenko@...labora.com" <dmitry.osipenko@...labora.com>,
        "kda@...ux-powerpc.org" <kda@...ux-powerpc.org>,
        "naveen.n.rao@...ux.vnet.ibm.com" <naveen.n.rao@...ux.vnet.ibm.com>
CC:     "linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] powerpc: replace ternary operator with min()



Le 24/10/2022 à 06:33, Russell Currey a écrit :
> On Sun, 2022-10-23 at 20:44 +0800, KaiLong Wang wrote:
>> Fix the following coccicheck warning:
>>
>> arch/powerpc/xmon/xmon.c:2987: WARNING opportunity for min()
>> arch/powerpc/xmon/xmon.c:2583: WARNING opportunity for min()
>>
>> Signed-off-by: KaiLong Wang <wangkailong@...i.cn>
> 
> Hello,
> 
> This fails to compile on some platforms/compilers since n is a long and
> 16 is an int, expanding to:
> 
> r = __builtin_choose_expr(
> 	((!!(sizeof((typeof(n) *)1 == (typeof(16) *)1))) &&
> 	 ((sizeof(int) ==
> 	   sizeof(*(8 ? ((void *)((long)(n)*0l)) : (int *)8))) &&
> 	  (sizeof(int) ==
> 	   sizeof(*(8 ? ((void *)((long)(16) * 0l)) :
> 			(int *)8))))),
> 	((n) < (16) ? (n) : (16)), ({
> 		typeof(n) __UNIQUE_ID___x0 = (n);
> 		typeof(16) __UNIQUE_ID___y1 = (16);
> 		((__UNIQUE_ID___x0) < (__UNIQUE_ID___y1) ?
> 			 (__UNIQUE_ID___x0) :
> 			 (__UNIQUE_ID___y1));
> 	}));
> 
> Here's the full build failure as found by snowpatch:
> https://github.com/ruscur/linux-ci/actions/runs/3308880562/jobs/5461579048#step:4:89
> 
> You should use min_t(long, n, 16) instead.

Wouldn't it work with 16L instead of 16 :

	min(n, 16L)

Christophe

> 
> - Russell
> 
>> ---
>>   arch/powerpc/xmon/xmon.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
>> index f51c882bf902..a7751cd2cc9d 100644
>> --- a/arch/powerpc/xmon/xmon.c
>> +++ b/arch/powerpc/xmon/xmon.c
>> @@ -2580,7 +2580,7 @@ static void xmon_rawdump (unsigned long adrs,
>> long ndump)
>>          unsigned char temp[16];
>>   
>>          for (n = ndump; n > 0;) {
>> -               r = n < 16? n: 16;
>> +               r = min(n, 16);
>>                  nr = mread(adrs, temp, r);
>>                  adrs += nr;
>>                  for (m = 0; m < r; ++m) {
>> @@ -2984,7 +2984,7 @@ prdump(unsigned long adrs, long ndump)
>>          for (n = ndump; n > 0;) {
>>                  printf(REG, adrs);
>>                  putchar(' ');
>> -               r = n < 16? n: 16;
>> +               r = min(n, 16);
>>                  nr = mread(adrs, temp, r);
>>                  adrs += nr;
>>                  for (m = 0; m < r; ++m) {
>