diff mbox series

cifs: remove redundant variable assignment

Message ID 20240313150034.165152-1-bharathsm@microsoft.com (mailing list archive)
State New, archived
Headers show
Series cifs: remove redundant variable assignment | expand

Commit Message

Bharath SM March 13, 2024, 3 p.m. UTC
This removes an unnecessary variable assignment. The assigned
value will be overwritten by cifs_fattr_to_inode before it
is accessed, making the line redundant.

Signed-off-by: Bharath SM <bharathsm@microsoft.com>
---
 fs/smb/client/inode.c | 2 --
 1 file changed, 2 deletions(-)

Comments

Steve French March 14, 2024, 2:49 a.m. UTC | #1
The second change in this looks a few lines off (didn't you mean to
remove the rc = 0 nine lines earlier, ie the one from the EREMOTE not
the EINVAL calse?).  See below:

        case -EREMOTE:
                cifs_create_junction_fattr(&fattr, inode->i_sb);
                rc = 0;   /* FIX: shouldn't you remove this one */
                break;
        case -EOPNOTSUPP:
        case -EINVAL:
                /*
                 * FIXME: legacy server -- fall back to path-based call?
                 * for now, just skip revalidating and mark inode for
                 * immediate reval.
                 */
-               rc = 0;   /* FIX: and not remove this one ? */
                CIFS_I(inode)->time = 0;
                goto cgfi_exit;
        default:
                goto cgfi_exit;
        }

        /*
         * don't bother with SFU junk here -- just mark inode as needing
         * revalidation.
         */
        fattr.cf_uniqueid = CIFS_I(inode)->uniqueid;
        fattr.cf_flags |= CIFS_FATTR_NEED_REVAL;
        /* if filetype is different, return error */
        rc = cifs_fattr_to_inode(inode, &fattr, false);


On Wed, Mar 13, 2024 at 10:01 AM Bharath SM <bharathsm.hsk@gmail.com> wrote:
>
> This removes an unnecessary variable assignment. The assigned
> value will be overwritten by cifs_fattr_to_inode before it
> is accessed, making the line redundant.
>
> Signed-off-by: Bharath SM <bharathsm@microsoft.com>
> ---
>  fs/smb/client/inode.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
> index 00aae4515a09..50e939234a8e 100644
> --- a/fs/smb/client/inode.c
> +++ b/fs/smb/client/inode.c
> @@ -400,7 +400,6 @@ cifs_get_file_info_unix(struct file *filp)
>                 cifs_unix_basic_to_fattr(&fattr, &find_data, cifs_sb);
>         } else if (rc == -EREMOTE) {
>                 cifs_create_junction_fattr(&fattr, inode->i_sb);
> -               rc = 0;
>         } else
>                 goto cifs_gfiunix_out;
>
> @@ -852,7 +851,6 @@ cifs_get_file_info(struct file *filp)
>                  * for now, just skip revalidating and mark inode for
>                  * immediate reval.
>                  */
> -               rc = 0;
>                 CIFS_I(inode)->time = 0;
>                 goto cgfi_exit;
>         default:
> --
> 2.34.1
>
Bharath SM March 14, 2024, 6:14 p.m. UTC | #2
Thanks for pointing it out. Updated patch.

On Thu, Mar 14, 2024 at 8:19 AM Steve French <smfrench@gmail.com> wrote:
>
> The second change in this looks a few lines off (didn't you mean to
> remove the rc = 0 nine lines earlier, ie the one from the EREMOTE not
> the EINVAL calse?).  See below:
>
>         case -EREMOTE:
>                 cifs_create_junction_fattr(&fattr, inode->i_sb);
>                 rc = 0;   /* FIX: shouldn't you remove this one */
>                 break;
>         case -EOPNOTSUPP:
>         case -EINVAL:
>                 /*
>                  * FIXME: legacy server -- fall back to path-based call?
>                  * for now, just skip revalidating and mark inode for
>                  * immediate reval.
>                  */
> -               rc = 0;   /* FIX: and not remove this one ? */
>                 CIFS_I(inode)->time = 0;
>                 goto cgfi_exit;
>         default:
>                 goto cgfi_exit;
>         }
>
>         /*
>          * don't bother with SFU junk here -- just mark inode as needing
>          * revalidation.
>          */
>         fattr.cf_uniqueid = CIFS_I(inode)->uniqueid;
>         fattr.cf_flags |= CIFS_FATTR_NEED_REVAL;
>         /* if filetype is different, return error */
>         rc = cifs_fattr_to_inode(inode, &fattr, false);
>
>
> On Wed, Mar 13, 2024 at 10:01 AM Bharath SM <bharathsm.hsk@gmail.com> wrote:
> >
> > This removes an unnecessary variable assignment. The assigned
> > value will be overwritten by cifs_fattr_to_inode before it
> > is accessed, making the line redundant.
> >
> > Signed-off-by: Bharath SM <bharathsm@microsoft.com>
> > ---
> >  fs/smb/client/inode.c | 2 --
> >  1 file changed, 2 deletions(-)
> >
> > diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
> > index 00aae4515a09..50e939234a8e 100644
> > --- a/fs/smb/client/inode.c
> > +++ b/fs/smb/client/inode.c
> > @@ -400,7 +400,6 @@ cifs_get_file_info_unix(struct file *filp)
> >                 cifs_unix_basic_to_fattr(&fattr, &find_data, cifs_sb);
> >         } else if (rc == -EREMOTE) {
> >                 cifs_create_junction_fattr(&fattr, inode->i_sb);
> > -               rc = 0;
> >         } else
> >                 goto cifs_gfiunix_out;
> >
> > @@ -852,7 +851,6 @@ cifs_get_file_info(struct file *filp)
> >                  * for now, just skip revalidating and mark inode for
> >                  * immediate reval.
> >                  */
> > -               rc = 0;
> >                 CIFS_I(inode)->time = 0;
> >                 goto cgfi_exit;
> >         default:
> > --
> > 2.34.1
> >
>
>
> --
> Thanks,
>
> Steve
diff mbox series

Patch

diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index 00aae4515a09..50e939234a8e 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -400,7 +400,6 @@  cifs_get_file_info_unix(struct file *filp)
 		cifs_unix_basic_to_fattr(&fattr, &find_data, cifs_sb);
 	} else if (rc == -EREMOTE) {
 		cifs_create_junction_fattr(&fattr, inode->i_sb);
-		rc = 0;
 	} else
 		goto cifs_gfiunix_out;
 
@@ -852,7 +851,6 @@  cifs_get_file_info(struct file *filp)
 		 * for now, just skip revalidating and mark inode for
 		 * immediate reval.
 		 */
-		rc = 0;
 		CIFS_I(inode)->time = 0;
 		goto cgfi_exit;
 	default: