diff mbox

[1/6] cifs: make error on lack of a unc= option more explicit

Message ID 1353087414-32152-2-git-send-email-jlayton@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jeff Layton Nov. 16, 2012, 5:36 p.m. UTC
Error out with a clear error message if there is no unc= option. The
existing code doesn't handle this in a clear fashion, and the check for
a UNCip option with no UNC string is just plain wrong.

Later, we'll fix the code to not require a unc= option, but for now we
need this to at least clarify why people are getting errors about DFS
parsing. With this change we can also get rid of some later NULL pointer
checks since we know the UNC and UNCip will never be NULL there.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/cifs/connect.c | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

Comments

Pavel Shilovsky Nov. 21, 2012, 3:12 p.m. UTC | #1
2012/11/16 Jeff Layton <jlayton@redhat.com>:
> Error out with a clear error message if there is no unc= option. The
> existing code doesn't handle this in a clear fashion, and the check for
> a UNCip option with no UNC string is just plain wrong.
>
> Later, we'll fix the code to not require a unc= option, but for now we
> need this to at least clarify why people are getting errors about DFS
> parsing. With this change we can also get rid of some later NULL pointer
> checks since we know the UNC and UNCip will never be NULL there.
>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
>  fs/cifs/connect.c | 19 +++++--------------
>  1 file changed, 5 insertions(+), 14 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 5c670b9..b9849ac 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -1799,6 +1799,11 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>                 goto cifs_parse_mount_err;
>         }
>  #endif
> +       if (!vol->UNC) {
> +               cERROR(1, "CIFS mount error: No UNC path (e.g. -o "
> +                       "unc=\\\\192.168.1.100\\public) specified");
> +               goto cifs_parse_mount_err;
> +       }
>
>         if (vol->UNCip == NULL)
>                 vol->UNCip = &vol->UNC[2];
> @@ -2070,17 +2075,6 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
>                         rc = -EINVAL;
>                         goto out_err;
>                 }
> -       } else if (volume_info->UNCip) {
> -               /* BB using ip addr as tcp_ses name to connect to the
> -                  DFS root below */
> -               cERROR(1, "Connecting to DFS root not implemented yet");
> -               rc = -EINVAL;
> -               goto out_err;
> -       } else /* which tcp_sess DFS root would we conect to */ {
> -               cERROR(1, "CIFS mount error: No UNC path (e.g. -o "
> -                       "unc=//192.168.1.100/public) specified");
> -               rc = -EINVAL;
> -               goto out_err;
>         }
>
>         /* see if we already have a matching tcp_ses */
> @@ -2736,9 +2730,6 @@ cifs_match_super(struct super_block *sb, void *data)
>
>         volume_info = mnt_data->vol;
>
> -       if (!volume_info->UNCip || !volume_info->UNC)
> -               goto out;
> -
>         rc = cifs_fill_sockaddr((struct sockaddr *)&addr,
>                                 volume_info->UNCip,
>                                 strlen(volume_info->UNCip),
> --
> 1.7.11.7
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Acked-by: Pavel Shilovsky <piastry@etersoft.ru>
Steve French Nov. 25, 2012, 9:30 p.m. UTC | #2
This looks fine but wondering if you plan to add support for mounting
to DFS root (which was supposed to be the only reason for allowing ip=
but not unc= to be specified)?

On Fri, Nov 16, 2012 at 11:36 AM, Jeff Layton <jlayton@redhat.com> wrote:
> Error out with a clear error message if there is no unc= option. The
> existing code doesn't handle this in a clear fashion, and the check for
> a UNCip option with no UNC string is just plain wrong.
>
> Later, we'll fix the code to not require a unc= option, but for now we
> need this to at least clarify why people are getting errors about DFS
> parsing. With this change we can also get rid of some later NULL pointer
> checks since we know the UNC and UNCip will never be NULL there.
>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
>  fs/cifs/connect.c | 19 +++++--------------
>  1 file changed, 5 insertions(+), 14 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 5c670b9..b9849ac 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -1799,6 +1799,11 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>                 goto cifs_parse_mount_err;
>         }
>  #endif
> +       if (!vol->UNC) {
> +               cERROR(1, "CIFS mount error: No UNC path (e.g. -o "
> +                       "unc=\\\\192.168.1.100\\public) specified");
> +               goto cifs_parse_mount_err;
> +       }
>
>         if (vol->UNCip == NULL)
>                 vol->UNCip = &vol->UNC[2];
> @@ -2070,17 +2075,6 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
>                         rc = -EINVAL;
>                         goto out_err;
>                 }
> -       } else if (volume_info->UNCip) {
> -               /* BB using ip addr as tcp_ses name to connect to the
> -                  DFS root below */
> -               cERROR(1, "Connecting to DFS root not implemented yet");
> -               rc = -EINVAL;
> -               goto out_err;
> -       } else /* which tcp_sess DFS root would we conect to */ {
> -               cERROR(1, "CIFS mount error: No UNC path (e.g. -o "
> -                       "unc=//192.168.1.100/public) specified");
> -               rc = -EINVAL;
> -               goto out_err;
>         }
>
>         /* see if we already have a matching tcp_ses */
> @@ -2736,9 +2730,6 @@ cifs_match_super(struct super_block *sb, void *data)
>
>         volume_info = mnt_data->vol;
>
> -       if (!volume_info->UNCip || !volume_info->UNC)
> -               goto out;
> -
>         rc = cifs_fill_sockaddr((struct sockaddr *)&addr,
>                                 volume_info->UNCip,
>                                 strlen(volume_info->UNCip),
> --
> 1.7.11.7
>
Jeff Layton Nov. 26, 2012, 1:07 a.m. UTC | #3
On Sun, 25 Nov 2012 15:30:18 -0600
Steve French <smfrench@gmail.com> wrote:

> This looks fine but wondering if you plan to add support for mounting
> to DFS root (which was supposed to be the only reason for allowing ip=
> but not unc= to be specified)?
> 

I have no plans to do so in the near future, but don't let me stop you
from doing so if you choose...
diff mbox

Patch

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 5c670b9..b9849ac 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -1799,6 +1799,11 @@  cifs_parse_mount_options(const char *mountdata, const char *devname,
 		goto cifs_parse_mount_err;
 	}
 #endif
+	if (!vol->UNC) {
+		cERROR(1, "CIFS mount error: No UNC path (e.g. -o "
+			"unc=\\\\192.168.1.100\\public) specified");
+		goto cifs_parse_mount_err;
+	}
 
 	if (vol->UNCip == NULL)
 		vol->UNCip = &vol->UNC[2];
@@ -2070,17 +2075,6 @@  cifs_get_tcp_session(struct smb_vol *volume_info)
 			rc = -EINVAL;
 			goto out_err;
 		}
-	} else if (volume_info->UNCip) {
-		/* BB using ip addr as tcp_ses name to connect to the
-		   DFS root below */
-		cERROR(1, "Connecting to DFS root not implemented yet");
-		rc = -EINVAL;
-		goto out_err;
-	} else /* which tcp_sess DFS root would we conect to */ {
-		cERROR(1, "CIFS mount error: No UNC path (e.g. -o "
-			"unc=//192.168.1.100/public) specified");
-		rc = -EINVAL;
-		goto out_err;
 	}
 
 	/* see if we already have a matching tcp_ses */
@@ -2736,9 +2730,6 @@  cifs_match_super(struct super_block *sb, void *data)
 
 	volume_info = mnt_data->vol;
 
-	if (!volume_info->UNCip || !volume_info->UNC)
-		goto out;
-
 	rc = cifs_fill_sockaddr((struct sockaddr *)&addr,
 				volume_info->UNCip,
 				strlen(volume_info->UNCip),