Re: [PATCH] cifs: call strtobool instead of custom implementation

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

 



On Mon, May 12, 2014 at 06:41:00AM -0400, Jeff Layton wrote:
> > This changes the returned value of cifs_linux_ext_proc_write() to
> > -EINVAL if strtobool() was not able to find a boolean value.
> > Previously setting anything non-bool wouldn't cause this side-effect.
> > 
> > Steve, is it OK to you? This is certainly a visible change in that
> > writing to a proc entry will cause user-space error where it was
> > ignored before.
> > 
> 
> I think returning an error there is the right thing to do. If someone
> is writing to this file, they likely mean for it to have some sort of
> an effect.
> 
> Honestly though, the *best* thing would be to (gradually) convert these
> procfile switches into module options. The way it stands now, it's
> really hard to set these at boot time. You have to:
> 
>     # modprobe cifs
>     # echo N > /proc/fs/cifs/LinuxExtensionsEnabled
>     # mount -t cifs ...
> 
> Making sure that gets done for anything in /etc/fstab is...tricky.
> 
> What would be best is to add module options that affect these
> variables that could be set via /etc/modprobe.d.
> 
> Then, either add a warning printk when someone writes to these files
> that they'll soon be deprecated (in 2-3 releases), or maybe turn them
> into symlinks that point to the files under /sys/module/cifs/parameters.
I agree.

I think strtobool() patches are fine (+ change to the commit message).
The change to module options should certainly be done in a separate
patchset.

-- 
/ Alexander Bokovoy
--
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