On Mon, 15 Nov 2010 20:24:41 +0530
Suresh Jayaraman <sjayaraman@xxxxxxx> wrote:
> On 11/15/2010 06:55 PM, Jeff Layton wrote:
> > On Mon, 15 Nov 2010 18:15:23 +0530
> > Suresh Jayaraman <sjayaraman@xxxxxxx> 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 <sjayaraman@xxxxxxx>
> >> ---
> >> 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 <sjayaraman@xxxxxxx>
> ---
> 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 <jlayton@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
