Patchwork accept all supported values for dir_mode

login
register
mail settings
Submitter Scott Lovenberg
Date June 3, 2010, 6:39 a.m.
Message ID <1275547159-30008-1-git-send-email-scott.lovenberg@gmail.com>
Download mbox | patch
Permalink /patch/54454/
State New
Headers show

Comments

Scott Lovenberg - June 3, 2010, 6:39 a.m.
The option parsing function now accepts all values for 'dir_mode' that are supported by the kernel side code.

Signed-off-by: Scott Lovenberg <scott.lovenberg@gmail.com>
---
 mount.cifs.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Jeff Layton - June 6, 2010, 11:33 a.m.
On Thu,  3 Jun 2010 02:39:19 -0400
Scott Lovenberg <scott.lovenberg@gmail.com> wrote:

> The option parsing function now accepts all values for 'dir_mode' that are supported by the kernel side code.
> 
> Signed-off-by: Scott Lovenberg <scott.lovenberg@gmail.com>
> ---
>  mount.cifs.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/mount.cifs.c b/mount.cifs.c
> index 65754c0..21ce532 100644
> --- a/mount.cifs.c
> +++ b/mount.cifs.c
> @@ -812,7 +812,7 @@ static int parse_opt_token(const char *token)
>  		return OPT_FILE_MODE;
>  	if (strncmp(token, "dmask", 5) == 0)
>  		return OPT_DMASK;
> -	if (strncmp(token, "dir_mode", 8) == 0)
> +	if (strncmp(token, "dir_mode", 4) == 0 || strncmp(token, "dirm", 4) == 0)
				      ^^^^
Sigh. But I can confirm that this is similarly broken in the kernel
so we have little choice but to live with it here.

>  		return OPT_DIR_MODE;
>  	if (strncmp(token, "nosuid", 6) == 0)
>  		return OPT_NO_SUID;


Committed...
Scott Lovenberg - June 7, 2010, 4:20 a.m.
>
> > -     if (strncmp(token, "dir_mode", 8) == 0)
> > +     if (strncmp(token, "dir_mode", 4) == 0 || strncmp(token, "dirm", 4)
> == 0)
>                                       ^^^^
> Sigh. But I can confirm that this is similarly broken in the kernel
> so we have little choice but to live with it here.
>
> That's exactly how I felt about this fix.  Do you have any feelings on
fixing this on both sides to accept only the full strings?  It might cause
regressions for people, so the kernel patch will probably be pulled by
Redhat, Suse and Debian (and anyone else backporting kernel patches for long
term releases) maintainers.
It seems like the right thing to do, but if it's just going to cause extra
work and be yanked either way... well, that's one step forward, two
backwards.
Jeff Layton - June 7, 2010, 11:24 a.m.
On Mon, 7 Jun 2010 00:20:14 -0400
Scott Lovenberg <scott.lovenberg@gmail.com> wrote:

> >
> > > -     if (strncmp(token, "dir_mode", 8) == 0)
> > > +     if (strncmp(token, "dir_mode", 4) == 0 || strncmp(token, "dirm", 4)
> > == 0)
> >                                       ^^^^
> > Sigh. But I can confirm that this is similarly broken in the kernel
> > so we have little choice but to live with it here.
> >
> > That's exactly how I felt about this fix.  Do you have any feelings on
> fixing this on both sides to accept only the full strings?  It might cause
> regressions for people, so the kernel patch will probably be pulled by
> Redhat, Suse and Debian (and anyone else backporting kernel patches for long
> term releases) maintainers.
> It seems like the right thing to do, but if it's just going to cause extra
> work and be yanked either way... well, that's one step forward, two
> backwards.
> 
> 

I think we're stuck living with it like this. The maintenance burden on
this sort of thing is fairly low so it's not really a big deal. This is
a good reason why we have to consider user-visible interfaces very
carefully however.

Patch

diff --git a/mount.cifs.c b/mount.cifs.c
index 65754c0..21ce532 100644
--- a/mount.cifs.c
+++ b/mount.cifs.c
@@ -812,7 +812,7 @@  static int parse_opt_token(const char *token)
 		return OPT_FILE_MODE;
 	if (strncmp(token, "dmask", 5) == 0)
 		return OPT_DMASK;
-	if (strncmp(token, "dir_mode", 8) == 0)
+	if (strncmp(token, "dir_mode", 4) == 0 || strncmp(token, "dirm", 4) == 0)
 		return OPT_DIR_MODE;
 	if (strncmp(token, "nosuid", 6) == 0)
 		return OPT_NO_SUID;