linux-kernel - Re: [PATCH linux-next] serial: drop unneeded assignment

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <f48ea2ed-c2a8-da6c-1ce9-20718e7c1f71@kernel.org>
Date:   Mon, 23 Aug 2021 12:36:13 +0200
From:   Jiri Slaby <jirislaby@...nel.org>
To:     Christophe JAILLET <christophe.jaillet@...adoo.fr>,
        CGEL <cgel.zte@...il.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     linux-kernel@...r.kernel.org, Luo penghao <luo.penghao@....com.cn>,
        Zeal Robot <zealci@....com.cn>
Subject: Re: [PATCH linux-next] serial: drop unneeded assignment

On 21. 08. 21, 7:04, Christophe JAILLET wrote:
> Hi,
> 
> Le 21/08/2021 à 04:17, CGEL a écrit :
>> From: Luo penghao <luo.penghao@....com.cn>
>>
>> The first assignment is not used. In order to keep the code style
>> consistency of the whole file, the first 'offset' assignment should be
>> deleted.
>>
>> The clang_analyzer complains as follows:
>>
>> drivers/tty/nozomi.c:520:6: warning:
>> Although the value storedto 'offset' is used in the enclosing expression,
>> the value is never actually read from 'offset'.
>>
>> Reported-by: Zeal Robot <zealci@....com.cn>
>> Signed-off-by: Luo penghao <luo.penghao@....com.cn>
>> ---
>>   drivers/tty/nozomi.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/tty/nozomi.c b/drivers/tty/nozomi.c
>> index 0454c78..210a48f 100644
>> --- a/drivers/tty/nozomi.c
>> +++ b/drivers/tty/nozomi.c
>> @@ -517,7 +517,7 @@ static void nozomi_setup_memory(struct nozomi *dc)
>>       /* Ctrl dl configuration */
>>       dc->port[PORT_CTRL].dl_addr[CH_A] =
>> -                (offset += dc->config_table.dl_app2_len);
>> +                (offset + dc->config_table.dl_app2_len);
> 
> I guess that it has been written that way for consistency reasons with 
> previous lines.
> 
> Should it be changed, you have the same pattern at line 554 that should 
> be updated the same way.

RIght.

Provided the code is always over two lines already, wouldn't it be more 
readable to calculate the offset before each line, not inline. Like:
         /* Modem port dl configuration */
         dc->port[PORT_MDM].dl_addr[CH_A] = offset;
	offset += dc->config_table.dl_mdm_len1;
         dc->port[PORT_MDM].dl_addr[CH_B] = offset;
... // the last one being:
         /* Ctrl dl configuration */
	offset += dc->config_table.dl_app2_len;
         dc->port[PORT_CTRL].dl_addr[CH_A] = offset;

That would 1) remove the warning too, 2) make it less error-prone / more 
readable.

thanks,
-- 
js
suse labs