On 2024-12-03 11:06:16-0500, James Bottomley wrote:
> > 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
Not all users are using the macros to define their attributes.
(Nor do they want to)
These users would break with your suggestion.
Otherwise I agree.
Thomas
