Re: [PATCH v2 09/10] sysfs: bin_attribute: add const read/write callback variants

James Bottomley
2024-12-03T16:06:25Z

> diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
> index
> d17c473c1ef292875475bf3bdf62d07241c13882..d713a6445a6267145a7014f308d
> f3bb25b8c3287 100644
> --- a/include/linux/sysfs.h
> +++ b/include/linux/sysfs.h
> @@ -305,8 +305,12 @@ struct bin_attribute {
> struct address_space *(*f_mapping)(void);
> ssize_t (*read)(struct file *, struct kobject *, struct
> bin_attribute *,
> char *, loff_t, size_t);
> + ssize_t (*read_new)(struct file *, struct kobject *, const
> struct bin_attribute *,
> + char *, loff_t, size_t);
> ssize_t (*write)(struct file *, struct kobject *, struct
> bin_attribute *,
> char *, loff_t, size_t);
> + ssize_t (*write_new)(struct file *, struct kobject *,
> + const struct bin_attribute *, char *,
> loff_t, size_t);
> loff_t (*llseek)(struct file *, struct kobject *, const
> struct bin_attribute *,
> loff_t, int);
> int (*mmap)(struct file *, struct kobject *, const struct
> bin_attribute *attr,
> @@ -325,11 +329,28 @@ struct bin_attribute {
> */
> #define sysfs_bin_attr_init(bin_attr) sysfs_attr_init(&(bin_attr)-
> >attr)
>
> +typedef ssize_t __sysfs_bin_rw_handler_new(struct file *, struct
> kobject *,
> + const struct
> bin_attribute *, char *, loff_t, size_t);
> +
> /* macros to create static binary attributes easier */
> #define __BIN_ATTR(_name, _mode, _read, _write, _size)
> { \
> .attr = { .name = __stringify(_name), .mode = _mode
> }, \
> - .read =
> _read, \
> - .write =
> _write, \
> + .read =
> _Generic(_read, \
> + __sysfs_bin_rw_handler_new * :
> NULL, \
> + default :
> _read \
> + ),
> \
> + .read_new =
> _Generic(_read, \
> + __sysfs_bin_rw_handler_new * :
> _read, \
> + default :
> NULL \
> + ),
> \
> + .write =
> _Generic(_write, \
> + __sysfs_bin_rw_handler_new * :
> NULL, \
> + default :
> _write \
> + ),
> \
> + .write_new =
> _Generic(_write, \
> + __sysfs_bin_rw_handler_new * :
> _write, \
> + default :
> NULL \
> + ),
> \
> .size =
> _size, \
> }

It's probably a bit late now, but you've done this the wrong way
around. What you should have done is added the const to .read/.write
then added a .read_old/.write_old with the original function prototype
and used _Generic() to switch between them. Then when there are no
more non const left, you can simply remove .read_old and .write_old
without getting Linus annoyed by having to do something like this:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e70140ba0d2b1a30467d4af6bcfe761327b9ec95

Regards,

James