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 <[email protected]>
---
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
Subsystem-Specific Callbacks
~~~~~~~~~~~~~~~~~~~~~~~~~~~~
--
2.1.4
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 <[email protected]>
> ---
>
> 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