From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752005AbbIHTbD (ORCPT ); Tue, 8 Sep 2015 15:31:03 -0400 Received: from bh-25.webhostbox.net ([208.91.199.152]:51364 "EHLO bh-25.webhostbox.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751136AbbIHTa6 (ORCPT ); Tue, 8 Sep 2015 15:30:58 -0400 Date: Tue, 8 Sep 2015 12:30:52 -0700 From: Guenter Roeck To: Greg KH Cc: Emilio =?iso-8859-1?Q?L=F3pez?= , olof@lixom.net, kgene@kernel.org, k.kozlowski@samsung.com, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org Subject: Re: [PATCH 1/3] sysfs: Fix is_visible() support for binary attributes Message-ID: <20150908193052.GA11106@roeck-us.net> References: <1441714066-5599-1-git-send-email-emilio.lopez@collabora.co.uk> <1441714066-5599-2-git-send-email-emilio.lopez@collabora.co.uk> <20150908153013.GA6758@roeck-us.net> <20150908191002.GB10156@kroah.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20150908191002.GB10156@kroah.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-Authenticated_sender: guenter@roeck-us.net X-OutGoing-Spam-Status: No, score=-1.0 X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - bh-25.webhostbox.net X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - roeck-us.net X-Get-Message-Sender-Via: bh-25.webhostbox.net: authenticated_id: guenter@roeck-us.net X-Source: X-Source-Args: X-Source-Dir: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Sep 08, 2015 at 12:10:02PM -0700, Greg KH wrote: > On Tue, Sep 08, 2015 at 08:30:13AM -0700, Guenter Roeck wrote: > > 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 > > > --- > > > 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. > > I agree, make i the number of the bin attribute and that should solve > this issue. > No, that would conflict with the "normal" use of is_visible for non-binary attributes, and make the index all but useless, since the is_visible function would have to search through all the attributes anyway to figure out which one is being checked. Guenter