Hello!
On 4/11/22 2:47 AM, Damien Le Moal wrote:
>> sil680_sel{dev|reg}() return a PCI config space address but needlessly
>> use the *unsigned long* type for that, whereas the PCI config space
>> accessors take *int* for the address parameter. Switch these functions
>> to returning *int*, updating the local variables at their call sites.
>> Add the empty lines after some declarations, while at it...
>>
>> Found by Linux Verification Center (linuxtesting.org) with the SVACE static
>> analysis tool.
>>
>> Signed-off-by: Sergey Shtylyov <s.shtylyov@xxxxxx>
>>
>> ---
>> This patch is against the 'for-next' branch of Damien Le Moal's 'libata.git'
>> repo.
>>
>> drivers/ata/pata_sil680.c | 27 +++++++++++++++------------
>> 1 file changed, 15 insertions(+), 12 deletions(-)
>>
>> Index: libata/drivers/ata/pata_sil680.c
>> ===================================================================
>> --- libata.orig/drivers/ata/pata_sil680.c
>> +++ libata/drivers/ata/pata_sil680.c
>> @@ -47,9 +47,10 @@
>> * criticial.
>> */
>>
>> -static unsigned long sil680_selreg(struct ata_port *ap, int r)
>> +static int sil680_selreg(struct ata_port *ap, int r)
>> {
>> - unsigned long base = 0xA0 + r;
>> + int base = 0xA0 + r;
>> +
>> base += (ap->port_no << 4);
>> return base;
>
> The variable "base" is rather useless here... A simple:
>
> return 0xA0 + r + (ap->port_no << 4);
>
> would work too and is a lot cleaner.
Yes, probably... but it's a matter of a separate patch, I think.
Note that both functions are inlined by gcc.
[...]
MBR, Sergey
