linux-kernel - Re: [PATCH][next] cifs: remove redundant variable tcon

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: <CANT5p=r=y4BN-1eeSur_zWCx=R1fn+8OU-3nv+AffraPjja+qQ@mail.gmail.com>
Date: Wed, 17 Jan 2024 08:07:17 +0530
From: Shyam Prasad N <nspmangalore@...il.com>
To: Steve French <smfrench@...il.com>
Cc: Colin Ian King <colin.i.king@...il.com>, Steve French <sfrench@...ba.org>, 
	Paulo Alcantara <pc@...guebit.com>, Shyam Prasad N <sprasad@...rosoft.com>, Tom Talpey <tom@...pey.com>, 
	linux-cifs@...r.kernel.org, samba-technical@...ts.samba.org, 
	kernel-janitors@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH][next] cifs: remove redundant variable tcon_exist

On Wed, Jan 17, 2024 at 4:47 AM Steve French <smfrench@...il.com> wrote:
>
> Yes - it looks like Shyam's commit made that variable obsolete.
>
> Shyam/Paulo,
> Let me know if any objections.  Will put into cifs-2.6.git for-next
>
> commit 04909192ada3285070f8ced0af7f07735478b364 (tag: 6.7-rc4-smb3-client-fixes)
> Author: Shyam Prasad N <sprasad@...rosoft.com>
> Date:   Wed Dec 6 16:37:38 2023 +0000
>
>     cifs: reconnect worker should take reference on server struct
> unconditionally
>
>     Reconnect worker currently assumes that the server struct
>     is alive and only takes reference on the server if it needs
>     to call smb2_reconnect.
>
>     With the new ability to disable channels based on whether the
>     server has multichannel disabled, this becomes a problem when
>     we need to disable established channels. While disabling the
>     channels and deallocating the server, there could be reconnect
>     work that could not be cancelled (because it started).
>
>     This change forces the reconnect worker to unconditionally
>     take a reference on the server when it runs.
>
>     Also, this change now allows smb2_reconnect to know if it was
>     called by the reconnect worker. Based on this, the cifs_put_tcp_session
>     can decide whether it can cancel the reconnect work synchronously or not.
>
>     Signed-off-by: Shyam Prasad N <sprasad@...rosoft.com>
>     Signed-off-by: Steve French <stfrench@...rosoft.com>
>
> On Tue, Jan 16, 2024 at 4:51 AM Colin Ian King <colin.i.king@...il.com> wrote:
> >
> > The variable tcon_exist is being assigned however it is never read, the
> > variable is redundant and can be removed.
> >
> > Cleans up clang scan build warning:
> > warning: Although the value stored to 'tcon_exist' is used in
> > the enclosing expression, the value is never actually readfrom
> > 'tcon_exist' [deadcode.DeadStores]
> >
> > Signed-off-by: Colin Ian King <colin.i.king@...il.com>
> > ---
> >  fs/smb/client/smb2pdu.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
> > index bd25c34dc398..50f6bf16b624 100644
> > --- a/fs/smb/client/smb2pdu.c
> > +++ b/fs/smb/client/smb2pdu.c
> > @@ -3918,7 +3918,7 @@ void smb2_reconnect_server(struct work_struct *work)
> >         struct cifs_ses *ses, *ses2;
> >         struct cifs_tcon *tcon, *tcon2;
> >         struct list_head tmp_list, tmp_ses_list;
> > -       bool tcon_exist = false, ses_exist = false;
> > +       bool ses_exist = false;
> >         bool tcon_selected = false;
> >         int rc;
> >         bool resched = false;
> > @@ -3964,7 +3964,7 @@ void smb2_reconnect_server(struct work_struct *work)
> >                         if (tcon->need_reconnect || tcon->need_reopen_files) {
> >                                 tcon->tc_count++;
> >                                 list_add_tail(&tcon->rlist, &tmp_list);
> > -                               tcon_selected = tcon_exist = true;
> > +                               tcon_selected = true;
> >                         }
> >                 }
> >                 /*
> > @@ -3973,7 +3973,7 @@ void smb2_reconnect_server(struct work_struct *work)
> >                  */
> >                 if (ses->tcon_ipc && ses->tcon_ipc->need_reconnect) {
> >                         list_add_tail(&ses->tcon_ipc->rlist, &tmp_list);
> > -                       tcon_selected = tcon_exist = true;
> > +                       tcon_selected = true;
> >                         cifs_smb_ses_inc_refcount(ses);
> >                 }
> >                 /*
> > --
> > 2.39.2
> >
> >
>
>
> --
> Thanks,
>
> Steve
>

The change looks good to me.

-- 
Regards,
Shyam