On Thu, Mar 1, 2018 at 7:19 AM, Jeff Layton <jlayton@xxxxxxxxx> wrote: > 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. > Right, so, allow me to explain why I wrote the option parsing bit this way. A bit of history is in order; when I first move the mount.cifs option parsing into its own function, the kernel side of the house wasn't using the standard kernel option parser. We had a bunch of string compares in an ever expanding "if...then...else if..." block in both the user space and the kernel space. Allowing for anything that started with "pass" was, IIRC, a trade off since we allowed credentials files to use "creds", etc. Now, in the manual we explicitly supported "password", but implicitly allowed the "pass" bit since there were other "shorthand" flags (off the top of my head... and this is going back the better part of a decade now... "creds", "dom", "user", etc) that were implicitly allowed in the same manner. My rational for the implicitly allowing shorthand versions of the flags was due to the fact that really, we're parsing to get an enum value - some options have multiple flags that collapse into the same OPT_ enum value and those were legacy flags we had to support in a backwards compatible way. If you _really_ need to start a new flag with "pass", you'd obviously just check for the longer string before checking for the shorter sub-string. This makes the kernel side mostly a moot point since you could accept "passwd" in the mount.cifs helper and give the kernel side whatever string you'd like since "pass" and "passwd" collapse into OPT_PASS which is then used to setup the struct that gets sent over to the kernel. Would I design it this way again? Maybe. But what we've got is still, IMHO, much more readable and maintainable than the ever expanding inline string parsing that we had previously and has enough flexibility to allow a few ways of parsing the options from the command line. I hope that at least shines a bit of light on the history / intent of the option parser for mount.cifs. -- 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
