LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH linux-next] serial: drop unneeded assignment
@ 2021-08-21  2:17 CGEL
  2021-08-21  5:04 ` Christophe JAILLET
  2021-08-23 10:40 ` Jiri Slaby
  0 siblings, 2 replies; 4+ messages in thread
From: CGEL @ 2021-08-21  2:17 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Luo penghao, Zeal Robot

From: Luo penghao <luo.penghao@zte.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@zte.com.cn>
Signed-off-by: Luo penghao <luo.penghao@zte.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);
 	dc->port[PORT_CTRL].dl_size[CH_A] =
 				dc->config_table.dl_ctrl_len - buff_offset;
 
-- 
2.15.2



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH linux-next] serial: drop unneeded assignment
  2021-08-21  2:17 [PATCH linux-next] serial: drop unneeded assignment CGEL
@ 2021-08-21  5:04 ` Christophe JAILLET
  2021-08-23 10:36   ` Jiri Slaby
  2021-08-23 10:40 ` Jiri Slaby
  1 sibling, 1 reply; 4+ messages in thread
From: Christophe JAILLET @ 2021-08-21  5:04 UTC (permalink / raw)
  To: CGEL, Greg Kroah-Hartman
  Cc: Jiri Slaby, linux-kernel, Luo penghao, Zeal Robot

Hi,

Le 21/08/2021 à 04:17, CGEL a écrit :
> From: Luo penghao <luo.penghao@zte.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@zte.com.cn>
> Signed-off-by: Luo penghao <luo.penghao@zte.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.

CJ

>   	dc->port[PORT_CTRL].dl_size[CH_A] =
>   				dc->config_table.dl_ctrl_len - buff_offset;
>   
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH linux-next] serial: drop unneeded assignment
  2021-08-21  5:04 ` Christophe JAILLET
@ 2021-08-23 10:36   ` Jiri Slaby
  0 siblings, 0 replies; 4+ messages in thread
From: Jiri Slaby @ 2021-08-23 10:36 UTC (permalink / raw)
  To: Christophe JAILLET, CGEL, Greg Kroah-Hartman
  Cc: linux-kernel, Luo penghao, Zeal Robot

On 21. 08. 21, 7:04, Christophe JAILLET wrote:
> Hi,
> 
> Le 21/08/2021 à 04:17, CGEL a écrit :
>> From: Luo penghao <luo.penghao@zte.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@zte.com.cn>
>> Signed-off-by: Luo penghao <luo.penghao@zte.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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH linux-next] serial: drop unneeded assignment
  2021-08-21  2:17 [PATCH linux-next] serial: drop unneeded assignment CGEL
  2021-08-21  5:04 ` Christophe JAILLET
@ 2021-08-23 10:40 ` Jiri Slaby
  1 sibling, 0 replies; 4+ messages in thread
From: Jiri Slaby @ 2021-08-23 10:40 UTC (permalink / raw)
  To: CGEL, Greg Kroah-Hartman; +Cc: linux-kernel, Luo penghao, Zeal Robot

One more hint:
$ git log --oneline  drivers/tty/nozomi.c|grep -i serial
<empty>

Please use some of already used "tty: nozomi:", "tty/nozomi:", or "tty: 
nozomi," prefixes for the subject.

On 21. 08. 21, 4:17, CGEL wrote:
> From: Luo penghao <luo.penghao@zte.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@zte.com.cn>
> Signed-off-by: Luo penghao <luo.penghao@zte.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);
>   	dc->port[PORT_CTRL].dl_size[CH_A] =
>   				dc->config_table.dl_ctrl_len - buff_offset;
>   
> 


-- 
js

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-08-23 10:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-21  2:17 [PATCH linux-next] serial: drop unneeded assignment CGEL
2021-08-21  5:04 ` Christophe JAILLET
2021-08-23 10:36   ` Jiri Slaby
2021-08-23 10:40 ` Jiri Slaby

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).