Emilio, On Tue, Sep 08, 2015 at 09:07:44AM -0300, Emilio López wrote: > According to the sysfs header file: > > "The returned value will replace static permissions defined in > struct attribute or struct bin_attribute." > > but this isn't the case, as is_visible is only called on > struct attribute only. This patch adds the code paths required > to support is_visible() on binary attributes. > > Signed-off-by: Emilio López <emilio.lopez@xxxxxxxxxxxxxxx> > --- > fs/sysfs/group.c | 22 ++++++++++++++++++---- > 1 file changed, 18 insertions(+), 4 deletions(-) > > diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c > index 39a0199..eb6996a 100644 > --- a/fs/sysfs/group.c > +++ b/fs/sysfs/group.c > @@ -37,10 +37,10 @@ static int create_files(struct kernfs_node *parent, struct kobject *kobj, > { > struct attribute *const *attr; > struct bin_attribute *const *bin_attr; > - int error = 0, i; > + int error = 0, i = 0; > > if (grp->attrs) { > - for (i = 0, attr = grp->attrs; *attr && !error; i++, attr++) { > + for (attr = grp->attrs; *attr && !error; i++, attr++) { > umode_t mode = (*attr)->mode; > > /* > @@ -73,13 +73,27 @@ static int create_files(struct kernfs_node *parent, struct kobject *kobj, > } > > if (grp->bin_attrs) { > - for (bin_attr = grp->bin_attrs; *bin_attr; bin_attr++) { > + for (bin_attr = grp->bin_attrs; *bin_attr; i++, bin_attr++) { > + umode_t mode = (*bin_attr)->attr.mode; > + > if (update) > kernfs_remove_by_name(parent, > (*bin_attr)->attr.name); > + if (grp->is_visible) { > + mode = grp->is_visible(kobj, > + &(*bin_attr)->attr, i); With this, if 'n' is the number of non-binary attributes, for i < n: The index passed to is_visible points to a non-binary attribute. for i >= n: The index passed to is_visible points to the (index - n)th binary attribute. Unless I am missing something, this is not explained anywhere, but it is not entirely trivial to understand. I think it should be documented. Also, it might be a good idea to check through existing code to ensure that this change doesn't accidentially cause trouble (existing drivers implementing both attribute types may not expect to see an index variable pointing to a value larger than the number of elements in the attrs array). Thanks, Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html