On Mon, 15 Nov 2010 20:24:41 +0530 Suresh Jayaraman <[email protected]> wrote:
> On 11/15/2010 06:55 PM, Jeff Layton wrote: > > On Mon, 15 Nov 2010 18:15:23 +0530 > > Suresh Jayaraman <[email protected]> wrote: > > > >> cifs_get_tcon() could return any of the following errors: > >> -ENOMEM/-ENODEV/EREMOTEIO. We should follow the DFS referral code path only > >> when we get -EREMOTEIO (STATUS_PATH_NOT_COVERED) from the server and not > >> otherwise. > >> > >> Compile-tested only. > >> > >> Signed-off-by: Suresh Jayaraman <[email protected]> > >> --- > >> fs/cifs/connect.c | 5 ++++- > >> 1 files changed, 4 insertions(+), 1 deletions(-) > >> > >> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c > >> index 251a17c..c3a2323 100644 > >> --- a/fs/cifs/connect.c > >> +++ b/fs/cifs/connect.c > >> @@ -2780,7 +2780,10 @@ try_mount_again: > >> if (IS_ERR(tcon)) { > >> rc = PTR_ERR(tcon); > >> tcon = NULL; > >> - goto remote_path_check; > >> + if (rc == -EREMOTEIO) > >> + goto remote_path_check; > >> + else > >> + goto mount_fail_check; > >> } > >> > >> /* do not care if following two calls succeed - informational */ > > > > Don't you mean "EREMOTE" here? I've always interpreted "EREMOTEIO" to > > Doh, yes. > > > mean that the server had the equivalent of an I/O error, whereas > > "EREMOTE" means "Object is remote". > > > > I don't see how this patch materially changes anything. Pseudocode, > > assuming that rc == -EREMOTE: > > I agree that this is not an improvement (more of a cleanup), but I think > it improves readability (given that the single function runs for more than > hundred lines), and skips a couple of unneeded checks. > > Here's a fixed version (but this patch can be ignored if we feel so): > > > cifs_get_tcon() could return any of the following errors: > -ENOMEM/-ENODEV/EREMOTEIO. We should follow the DFS referral code path only > when we get -EREMOTEIO (STATUS_PATH_NOT_COVERED) from the server and not > otherwise. > > Compile-tested only. > > Signed-off-by: Suresh Jayaraman <[email protected]> > --- > fs/cifs/connect.c | 5 ++++- > 1 files changed, 4 insertions(+), 1 deletions(-) > > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c > index 251a17c..6dbc145 100644 > --- a/fs/cifs/connect.c > +++ b/fs/cifs/connect.c > @@ -2780,7 +2780,10 @@ try_mount_again: > if (IS_ERR(tcon)) { > rc = PTR_ERR(tcon); > tcon = NULL; > - goto remote_path_check; > + if (rc == -EREMOTE) > + goto remote_path_check; > + else > + goto mount_fail_check; > } > > /* do not care if following two calls succeed - informational */ I just don't the point here. cifs_mount is quite messy, but I don't think this really does much to improve its readability. -- Jeff Layton <[email protected]> -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
