Re: [PATCH RFC] sysfs.txt: add note on available attribute macros

[Greg Kroah-Hartman]

On Thu, Feb 14, 2019 at 12:21:50PM +0100, Nicholas Mc Guire wrote:
> The common cases of attributes should probably be using the __ATTR_XXX
> macros to make code more concise and readable but the current sysfs.txt
> does not point developers to those convenience macros. Further there is
> no note in sysfs.txt currently explaining why trying to set a sysfs file
> to mode 0666 will fail.
> 
> Signed-off-by: Nicholas Mc Guire <hofrat@xxxxxxxxx>
> ---
> 
> Note quite sure if this is the right place to place the note on mode 0666 
> but it should be somewhere as any attempt to set 0666 will be reverted 
> to 0664.
> 
>  Documentation/filesystems/sysfs.txt | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/Documentation/filesystems/sysfs.txt b/Documentation/filesystems/sysfs.txt
> index 41411b0..c246142 100644
> --- a/Documentation/filesystems/sysfs.txt
> +++ b/Documentation/filesystems/sysfs.txt
> @@ -116,6 +116,22 @@ static struct device_attribute dev_attr_foo = {
>  	.store = store_foo,
>  };
>  
> +Note as stated in include/linux/kernel.h "OTHER_WRITABLE?  Generally
> +considered a bad idea." so trying to set a sysfs file writable for
> +everyone will fail reverting to mode 0664.
> +
> +For this common cases sysfs.h provides convenience macros to
> +make defining attributes easier as well as making code more
> +concise and readable. The above case could be shortened to:
> +
> +static struct device_attribute dev_attr_foo = __ATTR_RW(foo);
> +
> +the list of helpers available is:
> +__ATTR_RO(name): same as above with mode 0444
> +__ATTR_WO(name): same as above with mode 0200
> +__ATTR_RO_MODE(name, mode): allowing to pass in a specific mode
> +__ATTR_RW(name): setting mode to 0644
> +__ATTR_NULL: which sets the name to NULL

__ATTR_NULL is used to end a list of attributes, no one should actually
try to use it :)

And really, all the current users of it should be converted to use the
groups api instead, as it should not be needed.

__ATTR_RO_MODE() needs a bit more description as well.

So the idea of documenting these is great, just needs a bit of tweaking.

thanks,

greg k-h