Re: 答复: Re: 答复: [PATCH] mount.cifs: add a useful "passwd" option

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



The problem is that the kernel parser does not allow you to do options
in this way. It only matches entire option names, so we can't ever hope
to replicate this behavior there. In this case, we could do this because
we reinterpret the option string to "pass=XXXXXX" before sending them
down to the kernel.

Still, I think this is a bad precedent, because you've now blocked any
option string name that starts with "pass=". Also, if we ever wanted to
get rid of mount.cifs (and it is possible for simple setups to not use
it today), this works counter to that goal.

It sucks when someone's working setup breaks, but it's important to note
that we never documented that passwd= would ever do anything at all. The
fact that it worked before was a bug, IMO.


On Thu, 2018-03-01 at 10:11 +0800, zhang.xianwei8@xxxxxxxxxx wrote:
> Before this commit a1f3acd40b265f134a97a739a6898b3958d206b9, the
> options at the beginning of the string "pass" are valid.
> 
> -       if (strncmp(token, "pass", 4) == 0)
> +       if (strcmp(token, "pass") == 0 ||
> +               strcmp(token, "password") == 0)
>                 return OPT_PASS;
> 
> But, after that we can just use "pass" and "password", this causes some
> users can't use their old options such as "passwd":
> eg. https://www.raspberrypi.org/forums/viewtopic.php?t=191329
> The string "passwd" is a common option. So I think it is necessary to be supported.
> 
> Best regards,
> Zhang Xianwei
> 
> ------------------原始邮件------------------
> 发件人:SteveFrench <smfrench@xxxxxxxxx>
> 收件人:Jeff Layton <jlayton@xxxxxxxxx>
> 抄送人:张贤伟10129966;CIFS <linux-cifs@xxxxxxxxxxxxxxx>Pavel Shilovskiy <pshilov@xxxxxxxxxxxxx>
> 日 期 :2018年02月28日 23:24
> 主 题 :Re: 答复: [PATCH] mount.cifs: add a useful "passwd" option
> My general opinion on option synonyms is that they can add value when
> they match the names of options already used by similar tools (e.g.
> smbclient or nfs mount utilities) since that can remove confusion for
> users familiar with those tools - but we have to be careful since cifs
> exposes a large number of options on mount (due to all the dialects
> and server types).
> 
> On Wed, Feb 28, 2018 at 6:26 AM, Jeff Layton <jlayton@xxxxxxxxx> wrote:
> > On Wed, 2018-02-28 at 15:52 +0800, zhang.xianwei8@xxxxxxxxxx wrote:
> > > Dear all,
> > >     Could you take a little time to review it? Thanks:)
> > > 
> > > 
> > > ------------------原始邮件------------------
> > > 发件人:张贤伟10129966
> > > 收件人:jlayton@xxxxxxxxx <jlayton@xxxxxxxxx>
> > > 抄送人:linux-cifs@xxxxxxxxxxxxxxx <linux-cifs@xxxxxxxxxxxxxxx>smfrench@xxxxxxxxx <smfrench@xxxxxxxxx>pshilov@xxxxxxxxxxxxxx <pshilov@xxxxxxxxxxxxx>
> > > 日 期 :2018年02月14日 14:05
> > > 主 题 :[PATCH] mount.cifs: add a useful "passwd" option
> > > Add a useful "passwd" option and update mount.cifs manpage.
> > > 
> > > Signed-off-by: Zhang Xianwei
> > > ---
> > > mount.cifs.c | 1 +
> > > mount.cifs.rst | 2 +-
> > > 2 files changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/mount.cifs.c b/mount.cifs.c
> > > index ae7a899..aa5e6b0 100644
> > > --- a/mount.cifs.c
> > > +++ b/mount.cifs.c
> > > @@ -693,6 +693,7 @@ static int parse_opt_token(const char *token)
> > > strcmp(token, "username") == 0)
> > > return OPT_USER;
> > > if (strcmp(token, "pass") == 0 ||
> > > +     strcmp(token, "passwd") == 0 ||
> > > strcmp(token, "password") == 0)
> > > return OPT_PASS;
> > > if (strcmp(token, "sec") == 0)
> > > diff --git a/mount.cifs.rst b/mount.cifs.rst
> > > index a81c6c4..472ecb9 100644
> > > --- a/mount.cifs.rst
> > > +++ b/mount.cifs.rst
> > > @@ -64,7 +64,7 @@ username=arg|user=arg
> > > non-superuser mount. It is therefore recommended to use the full
> > > ``username=`` option name.
> > > 
> > > -password=arg|pass=arg
> > > +password=arg|pass=arg|passwd=arg
> > > specifies the CIFS password. If this option is not given then the
> > > environment variable PASSWD is used. If the password is not specified
> > > directly or indirectly via an argument to mount, mount.cifs will
> > 
> > 
> > I won't NAK this, as Pavel and Steve may feel differently, and they're
> > more involved with cifs these days than I am, but...
> > 
> > I'm not a fan of adding new option synonyms like this. They're a
> > maintenance burden, and they also cause confusing and conflicting usages
> > to be spread across the Internet in documentation, blog posts, etc.
> > 
> > We already have some aliased options like this in cifs, but in my
> > experience, it really is better to just have _one_ way to specify an
> > option.
> > --
> > Jeff Layton <jlayton@xxxxxxxxx>
> 
> 
> 
> --
> Thanks,
> 
> Steve

-- 
Jeff Layton <jlayton@xxxxxxxxx>
--
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



[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux