[2/3] cifs: make cifs_chan_update_iface() a void function

Message ID eac139a7-76d4-4067-8c25-15e30692aaf9@moroto.mountain
State New
Headers
Series [1/3] cifs: delete unnecessary NULL checks in cifs_chan_update_iface() |

Commit Message

Dan Carpenter Jan. 8, 2024, 9:08 a.m. UTC
  The return values for cifs_chan_update_iface() didn't match what the
documentation said and nothing was checking them anyway.  Just make it
a void function.

Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
 fs/smb/client/cifsproto.h |  2 +-
 fs/smb/client/sess.c      | 17 +++++++----------
 2 files changed, 8 insertions(+), 11 deletions(-)
  

Comments

Christophe JAILLET Jan. 8, 2024, 7:09 p.m. UTC | #1
Le 08/01/2024 à 10:08, Dan Carpenter a écrit :
> The return values for cifs_chan_update_iface() didn't match what the
> documentation said and nothing was checking them anyway.  Just make it
> a void function.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
>   fs/smb/client/cifsproto.h |  2 +-
>   fs/smb/client/sess.c      | 17 +++++++----------
>   2 files changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
> index afbab86331a1..a841bf4967fa 100644
> --- a/fs/smb/client/cifsproto.h
> +++ b/fs/smb/client/cifsproto.h
> @@ -656,7 +656,7 @@ cifs_chan_is_iface_active(struct cifs_ses *ses,
>   			  struct TCP_Server_Info *server);
>   void
>   cifs_disable_secondary_channels(struct cifs_ses *ses);
> -int
> +void
>   cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server);
>   int
>   SMB3_request_interfaces(const unsigned int xid, struct cifs_tcon *tcon, bool in_mount);
> diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c
> index 775c6a4a2f4b..f7b216dd06b2 100644
> --- a/fs/smb/client/sess.c
> +++ b/fs/smb/client/sess.c
> @@ -356,10 +356,9 @@ cifs_disable_secondary_channels(struct cifs_ses *ses)
>   
>   /*
>    * update the iface for the channel if necessary.
> - * will return 0 when iface is updated, 1 if removed, 2 otherwise
>    * Must be called with chan_lock held.
>    */
> -int
> +void
>   cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server)
>   {
>   	unsigned int chan_index;
> @@ -368,20 +367,19 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server)
>   	struct cifs_server_iface *old_iface = NULL;
>   	struct cifs_server_iface *last_iface = NULL;
>   	struct sockaddr_storage ss;
> -	int rc = 0;
>   
>   	spin_lock(&ses->chan_lock);
>   	chan_index = cifs_ses_get_chan_index(ses, server);
>   	if (chan_index == CIFS_INVAL_CHAN_INDEX) {
>   		spin_unlock(&ses->chan_lock);
> -		return 0;
> +		return;
>   	}
>   
>   	if (ses->chans[chan_index].iface) {
>   		old_iface = ses->chans[chan_index].iface;
>   		if (old_iface->is_active) {
>   			spin_unlock(&ses->chan_lock);
> -			return 1;
> +			return;
>   		}
>   	}
>   	spin_unlock(&ses->chan_lock);
> @@ -394,7 +392,7 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server)
>   	if (!ses->iface_count) {
>   		spin_unlock(&ses->iface_lock);
>   		cifs_dbg(VFS, "server %s does not advertise interfaces\n", ses->server->hostname);
> -		return 0;
> +		return;
>   	}
>   
>   	last_iface = list_last_entry(&ses->iface_list, struct cifs_server_iface,
> @@ -434,7 +432,6 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server)
>   	}
>   
>   	if (list_entry_is_head(iface, &ses->iface_list, iface_head)) {
> -		rc = 1;
>   		iface = NULL;
>   		cifs_dbg(FYI, "unable to find a suitable iface\n");
>   	}
> @@ -449,7 +446,7 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server)
>   		}
>   
>   		spin_unlock(&ses->iface_lock);
> -		return 0;
> +		return;
>   	}
>   
>   	/* now drop the ref to the current iface */
> @@ -478,13 +475,13 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server)
>   	chan_index = cifs_ses_get_chan_index(ses, server);
>   	if (chan_index == CIFS_INVAL_CHAN_INDEX) {
>   		spin_unlock(&ses->chan_lock);
> -		return 0;
> +		return;
>   	}
>   
>   	ses->chans[chan_index].iface = iface;
>   	spin_unlock(&ses->chan_lock);
>   
> -	return rc;
> +	return;

just remove this one?

CJ

>   }
>   
>   /*
  
Steve French Jan. 9, 2024, 4:41 a.m. UTC | #2
added the trivial change Christophe Suggested


On Mon, Jan 8, 2024 at 1:09 PM Christophe JAILLET
<christophe.jaillet@wanadoo.fr> wrote:
>
> Le 08/01/2024 à 10:08, Dan Carpenter a écrit :
> > The return values for cifs_chan_update_iface() didn't match what the
> > documentation said and nothing was checking them anyway.  Just make it
> > a void function.
> >
> > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> > ---
> >   fs/smb/client/cifsproto.h |  2 +-
> >   fs/smb/client/sess.c      | 17 +++++++----------
> >   2 files changed, 8 insertions(+), 11 deletions(-)
> >
> > diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
> > index afbab86331a1..a841bf4967fa 100644
> > --- a/fs/smb/client/cifsproto.h
> > +++ b/fs/smb/client/cifsproto.h
> > @@ -656,7 +656,7 @@ cifs_chan_is_iface_active(struct cifs_ses *ses,
> >                         struct TCP_Server_Info *server);
> >   void
> >   cifs_disable_secondary_channels(struct cifs_ses *ses);
> > -int
> > +void
> >   cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server);
> >   int
> >   SMB3_request_interfaces(const unsigned int xid, struct cifs_tcon *tcon, bool in_mount);
> > diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c
> > index 775c6a4a2f4b..f7b216dd06b2 100644
> > --- a/fs/smb/client/sess.c
> > +++ b/fs/smb/client/sess.c
> > @@ -356,10 +356,9 @@ cifs_disable_secondary_channels(struct cifs_ses *ses)
> >
> >   /*
> >    * update the iface for the channel if necessary.
> > - * will return 0 when iface is updated, 1 if removed, 2 otherwise
> >    * Must be called with chan_lock held.
> >    */
> > -int
> > +void
> >   cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server)
> >   {
> >       unsigned int chan_index;
> > @@ -368,20 +367,19 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server)
> >       struct cifs_server_iface *old_iface = NULL;
> >       struct cifs_server_iface *last_iface = NULL;
> >       struct sockaddr_storage ss;
> > -     int rc = 0;
> >
> >       spin_lock(&ses->chan_lock);
> >       chan_index = cifs_ses_get_chan_index(ses, server);
> >       if (chan_index == CIFS_INVAL_CHAN_INDEX) {
> >               spin_unlock(&ses->chan_lock);
> > -             return 0;
> > +             return;
> >       }
> >
> >       if (ses->chans[chan_index].iface) {
> >               old_iface = ses->chans[chan_index].iface;
> >               if (old_iface->is_active) {
> >                       spin_unlock(&ses->chan_lock);
> > -                     return 1;
> > +                     return;
> >               }
> >       }
> >       spin_unlock(&ses->chan_lock);
> > @@ -394,7 +392,7 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server)
> >       if (!ses->iface_count) {
> >               spin_unlock(&ses->iface_lock);
> >               cifs_dbg(VFS, "server %s does not advertise interfaces\n", ses->server->hostname);
> > -             return 0;
> > +             return;
> >       }
> >
> >       last_iface = list_last_entry(&ses->iface_list, struct cifs_server_iface,
> > @@ -434,7 +432,6 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server)
> >       }
> >
> >       if (list_entry_is_head(iface, &ses->iface_list, iface_head)) {
> > -             rc = 1;
> >               iface = NULL;
> >               cifs_dbg(FYI, "unable to find a suitable iface\n");
> >       }
> > @@ -449,7 +446,7 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server)
> >               }
> >
> >               spin_unlock(&ses->iface_lock);
> > -             return 0;
> > +             return;
> >       }
> >
> >       /* now drop the ref to the current iface */
> > @@ -478,13 +475,13 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server)
> >       chan_index = cifs_ses_get_chan_index(ses, server);
> >       if (chan_index == CIFS_INVAL_CHAN_INDEX) {
> >               spin_unlock(&ses->chan_lock);
> > -             return 0;
> > +             return;
> >       }
> >
> >       ses->chans[chan_index].iface = iface;
> >       spin_unlock(&ses->chan_lock);
> >
> > -     return rc;
> > +     return;
>
> just remove this one?
>
> CJ
>
> >   }
> >
> >   /*
>
>
  
Steve French Jan. 9, 2024, 4:42 a.m. UTC | #3
merged into cifs-2.6.git for-next

On Mon, Jan 8, 2024 at 3:08 AM Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> The return values for cifs_chan_update_iface() didn't match what the
> documentation said and nothing was checking them anyway.  Just make it
> a void function.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
>  fs/smb/client/cifsproto.h |  2 +-
>  fs/smb/client/sess.c      | 17 +++++++----------
>  2 files changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
> index afbab86331a1..a841bf4967fa 100644
> --- a/fs/smb/client/cifsproto.h
> +++ b/fs/smb/client/cifsproto.h
> @@ -656,7 +656,7 @@ cifs_chan_is_iface_active(struct cifs_ses *ses,
>                           struct TCP_Server_Info *server);
>  void
>  cifs_disable_secondary_channels(struct cifs_ses *ses);
> -int
> +void
>  cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server);
>  int
>  SMB3_request_interfaces(const unsigned int xid, struct cifs_tcon *tcon, bool in_mount);
> diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c
> index 775c6a4a2f4b..f7b216dd06b2 100644
> --- a/fs/smb/client/sess.c
> +++ b/fs/smb/client/sess.c
> @@ -356,10 +356,9 @@ cifs_disable_secondary_channels(struct cifs_ses *ses)
>
>  /*
>   * update the iface for the channel if necessary.
> - * will return 0 when iface is updated, 1 if removed, 2 otherwise
>   * Must be called with chan_lock held.
>   */
> -int
> +void
>  cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server)
>  {
>         unsigned int chan_index;
> @@ -368,20 +367,19 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server)
>         struct cifs_server_iface *old_iface = NULL;
>         struct cifs_server_iface *last_iface = NULL;
>         struct sockaddr_storage ss;
> -       int rc = 0;
>
>         spin_lock(&ses->chan_lock);
>         chan_index = cifs_ses_get_chan_index(ses, server);
>         if (chan_index == CIFS_INVAL_CHAN_INDEX) {
>                 spin_unlock(&ses->chan_lock);
> -               return 0;
> +               return;
>         }
>
>         if (ses->chans[chan_index].iface) {
>                 old_iface = ses->chans[chan_index].iface;
>                 if (old_iface->is_active) {
>                         spin_unlock(&ses->chan_lock);
> -                       return 1;
> +                       return;
>                 }
>         }
>         spin_unlock(&ses->chan_lock);
> @@ -394,7 +392,7 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server)
>         if (!ses->iface_count) {
>                 spin_unlock(&ses->iface_lock);
>                 cifs_dbg(VFS, "server %s does not advertise interfaces\n", ses->server->hostname);
> -               return 0;
> +               return;
>         }
>
>         last_iface = list_last_entry(&ses->iface_list, struct cifs_server_iface,
> @@ -434,7 +432,6 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server)
>         }
>
>         if (list_entry_is_head(iface, &ses->iface_list, iface_head)) {
> -               rc = 1;
>                 iface = NULL;
>                 cifs_dbg(FYI, "unable to find a suitable iface\n");
>         }
> @@ -449,7 +446,7 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server)
>                 }
>
>                 spin_unlock(&ses->iface_lock);
> -               return 0;
> +               return;
>         }
>
>         /* now drop the ref to the current iface */
> @@ -478,13 +475,13 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server)
>         chan_index = cifs_ses_get_chan_index(ses, server);
>         if (chan_index == CIFS_INVAL_CHAN_INDEX) {
>                 spin_unlock(&ses->chan_lock);
> -               return 0;
> +               return;
>         }
>
>         ses->chans[chan_index].iface = iface;
>         spin_unlock(&ses->chan_lock);
>
> -       return rc;
> +       return;
>  }
>
>  /*
> --
> 2.42.0
>
>
  
Dan Carpenter Jan. 9, 2024, 7:39 a.m. UTC | #4
On Mon, Jan 08, 2024 at 08:09:17PM +0100, Christophe JAILLET wrote:
> > @@ -478,13 +475,13 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server)
> >   	chan_index = cifs_ses_get_chan_index(ses, server);
> >   	if (chan_index == CIFS_INVAL_CHAN_INDEX) {
> >   		spin_unlock(&ses->chan_lock);
> > -		return 0;
> > +		return;
> >   	}
> >   	ses->chans[chan_index].iface = iface;
> >   	spin_unlock(&ses->chan_lock);
> > -	return rc;
> > +	return;
> 
> just remove this one?
> 

Doh.  Yeah.  I'll send a v2 of this tomorrow.

regards,
dan carpenter
  
Dan Carpenter Jan. 9, 2024, 7:44 a.m. UTC | #5
On Mon, Jan 08, 2024 at 10:41:35PM -0600, Steve French wrote:
> added the trivial change Christophe Suggested
> 

Thanks, Steve.

regards,
dan carpenter
  

Patch

diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
index afbab86331a1..a841bf4967fa 100644
--- a/fs/smb/client/cifsproto.h
+++ b/fs/smb/client/cifsproto.h
@@ -656,7 +656,7 @@  cifs_chan_is_iface_active(struct cifs_ses *ses,
 			  struct TCP_Server_Info *server);
 void
 cifs_disable_secondary_channels(struct cifs_ses *ses);
-int
+void
 cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server);
 int
 SMB3_request_interfaces(const unsigned int xid, struct cifs_tcon *tcon, bool in_mount);
diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c
index 775c6a4a2f4b..f7b216dd06b2 100644
--- a/fs/smb/client/sess.c
+++ b/fs/smb/client/sess.c
@@ -356,10 +356,9 @@  cifs_disable_secondary_channels(struct cifs_ses *ses)
 
 /*
  * update the iface for the channel if necessary.
- * will return 0 when iface is updated, 1 if removed, 2 otherwise
  * Must be called with chan_lock held.
  */
-int
+void
 cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server)
 {
 	unsigned int chan_index;
@@ -368,20 +367,19 @@  cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server)
 	struct cifs_server_iface *old_iface = NULL;
 	struct cifs_server_iface *last_iface = NULL;
 	struct sockaddr_storage ss;
-	int rc = 0;
 
 	spin_lock(&ses->chan_lock);
 	chan_index = cifs_ses_get_chan_index(ses, server);
 	if (chan_index == CIFS_INVAL_CHAN_INDEX) {
 		spin_unlock(&ses->chan_lock);
-		return 0;
+		return;
 	}
 
 	if (ses->chans[chan_index].iface) {
 		old_iface = ses->chans[chan_index].iface;
 		if (old_iface->is_active) {
 			spin_unlock(&ses->chan_lock);
-			return 1;
+			return;
 		}
 	}
 	spin_unlock(&ses->chan_lock);
@@ -394,7 +392,7 @@  cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server)
 	if (!ses->iface_count) {
 		spin_unlock(&ses->iface_lock);
 		cifs_dbg(VFS, "server %s does not advertise interfaces\n", ses->server->hostname);
-		return 0;
+		return;
 	}
 
 	last_iface = list_last_entry(&ses->iface_list, struct cifs_server_iface,
@@ -434,7 +432,6 @@  cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server)
 	}
 
 	if (list_entry_is_head(iface, &ses->iface_list, iface_head)) {
-		rc = 1;
 		iface = NULL;
 		cifs_dbg(FYI, "unable to find a suitable iface\n");
 	}
@@ -449,7 +446,7 @@  cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server)
 		}
 
 		spin_unlock(&ses->iface_lock);
-		return 0;
+		return;
 	}
 
 	/* now drop the ref to the current iface */
@@ -478,13 +475,13 @@  cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server)
 	chan_index = cifs_ses_get_chan_index(ses, server);
 	if (chan_index == CIFS_INVAL_CHAN_INDEX) {
 		spin_unlock(&ses->chan_lock);
-		return 0;
+		return;
 	}
 
 	ses->chans[chan_index].iface = iface;
 	spin_unlock(&ses->chan_lock);
 
-	return rc;
+	return;
 }
 
 /*