accept all supported values for dir_mode

Submitted by Scott Lovenberg on June 3, 2010, 6:39 a.m.

Details

Message ID 1275547159-30008-1-git-send-email-scott.lovenberg@gmail.com
State New
Headers show

Commit Message

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(-)

Comments

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 hide | download patch | download mbox

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;