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

[Jiri Slaby]

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