linux-kernel - Re: [PATCH v3 2/2] scsi: dc395x: improve code formatting for the macros

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: <c6a39939-b0cd-43a3-8b63-114909ba001e@cyano.uk>
Date: Tue, 23 Sep 2025 21:31:14 +0800
From: Xinhui Yang <cyan@...no.uk>
To: James Bottomley <James.Bottomley@...senPartnership.com>,
 linux-scsi@...r.kernel.org
Cc: stable@...r.kernel.org, Mingcong Bai <jeffbai@...c.io>,
 Kexy Biscuit <kexybiscuit@...c.io>, Oliver Neukum <oliver@...kum.org>,
 Ali Akcaagac <aliakc@....de>, Jamie Lenehan <lenehan@...bble.org>,
 "Martin K. Petersen" <martin.petersen@...cle.com>,
 open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 2/2] scsi: dc395x: improve code formatting for the
 macros

Hi James,

在 2025/9/23 21:09, James Bottomley 写道:
> On Tue, 2025-09-23 at 20:52 +0800, Xinhui Yang wrote:
>> These DC395x_* macros does not have white spaces around their
>> arguments,
>> thus checkpatch.pl throws an error for each change in the macros.
>>
>> Also, there are no surrounding parentheses in the expressions for the
>> read and write macros, which checkpatch.pl also complained about.
>>
>> This patch does only formatting improvements to make the macro
>> definitions align with the previous patch.
>>
>> Signed-off-by: Xinhui Yang <cyan@...no.uk>
>> ---
>>  drivers/scsi/dc395x.c | 16 ++++++++--------
>>  1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/scsi/dc395x.c b/drivers/scsi/dc395x.c
>> index aed4f21e8143..cff6fa20e53c 100644
>> --- a/drivers/scsi/dc395x.c
>> +++ b/drivers/scsi/dc395x.c
>> @@ -91,8 +91,8 @@
>>  #endif
>>  
>>  
>> -#define
>> DC395x_LOCK_IO(dev,flags)		spin_lock_irqsave(((struct Scsi_Host *)dev)->host_lock,flags)
>> -#define
>> DC395x_UNLOCK_IO(dev,flags)		spin_unlock_irqrestore(((struct Scsi_Host*)dev)->host_lock,flags)
>> +#define DC395x_LOCK_IO(dev,
>> flags)		spin_lock_irqsave(((struct Scsi_Host *)dev)->host_lock, flags)
>> +#define DC395x_UNLOCK_IO(dev,
>> flags)		spin_unlock_irqrestore(((struct Scsi_Host *)dev)->host_lock, flags)
>>  
>>  /*
>>   * read operations that may trigger side effects in the hardware,
>> @@ -100,12 +100,12 @@
>>   */
>>  #define DC395x_peek8(acb, address)		((void)(inb(acb-
>>> io_port_base + (address))))
>>  /* normal read write operations goes here. */
>> -#define DC395x_read8(acb,address)		(u8)(inb(acb-
>>> io_port_base + (address)))
>> -#define DC395x_read16(acb,address)		(u16)(inw(acb-
>>> io_port_base + (address)))
>> -#define DC395x_read32(acb,address)		(u32)(inl(acb-
>>> io_port_base + (address)))
>> -#define DC395x_write8(acb,address,value)	outb((value), acb-
>>> io_port_base + (address))
>> -#define DC395x_write16(acb,address,value)	outw((value), acb-
>>> io_port_base + (address))
>> -#define DC395x_write32(acb,address,value)	outl((value), acb-
>>> io_port_base + (address))
>> +#define DC395x_read8(acb, address)		((u8)    (inb(acb-
>>> io_port_base + (address))))
>> +#define DC395x_read16(acb, address)		((u16)   (inw(acb-
>>> io_port_base + (address))))
>> +#define DC395x_read32(acb, address)		((u32)   (inl(acb-
>>> io_port_base + (address))))
> 
> This doesn't look right.  The problem checkpatch is complaining about
> is surely that the cast makes it a compound statement.  However, since
> inb inw and inl all return the types they're being cast to the correct
> solution is surely to remove the cast making these single statements
> that don't need parentheses.

Thanks, I checked the definitions and you are right - these read macros
should have their casts removed, as they now return u8, u16 and u32
respectively.

>> +#define DC395x_write8(acb, address, value)	(outb((value), acb-
>>> io_port_base + (address)))
>> +#define DC395x_write16(acb, address, value)	(outw((value), acb-
>>> io_port_base + (address)))
>> +#define DC395x_write32(acb, address, value)	(outl((value), acb-
>>> io_port_base + (address)))
> 
> And these are single statements which shouldn't need parentheses.  Are
> you sure checkpatch is complaining about this, because if it is then
> checkpatch needs fixing.

Thanks for pointing this out, they don't need additional parentheses.

> Regards,
> 
> James
> 

Thanks,
Xinhui