linux-kernel - Re: [PATCH] sysfs: add filter function to groups

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 31 Oct 2007 03:01:56 +0100
From:	"Kay Sievers" <kay.sievers@...y.org>
To:	"Mark M. Hoffman" <mhoffman@...htlink.com>
Cc:	"James Bottomley" <James.Bottomley@...eleye.com>,
	"Stefan Richter" <stefanr@...6.in-berlin.de>,
	"Greg KH" <greg@...ah.com>,
	linux-scsi <linux-scsi@...r.kernel.org>,
	linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] sysfs: add filter function to groups

On Oct 31, 2007 1:40 AM, Mark M. Hoffman <mhoffman@...htlink.com> wrote:
> * James Bottomley <James.Bottomley@...elEye.com> [2007-10-30 13:25:43 -0500]:
> > On Mon, 2007-10-29 at 18:58 +0100, Stefan Richter wrote:
> > > James Bottomley wrote:
> > > >> >  struct attribute_group {
> > > >> >        const char              *name;
> > > >> > +      int                     (*filter_show)(struct kobject *, int);
> > >
> > > > Actually, it returns a true/false value indicating whether the given
> > > > attribute should be displayed.
> > >
> > > How about this:
> > >
> > >     int (*is_visible)(...);
> >
> > OK, so is this latest revision acceptable to everyone?
>
> I've just been hacking around in this area a bit, for a completely different
> reason: there are literally 1000's of attributes in drivers/hwmon/*.c that
> really want to be const, but which cannot be due to the current API.  I was
> about to propose some patches that move in a different direction...

That isn't related to "dynamic attributes", right?

> IMHO the fundamental problem is struct attribute_group itself.  This structure
> is nothing but a convenience for packaging the arguments to sysfs_create_group
> and sysfs_remove_group.

That "problem" is actually a good thing. If you look at the change
rate of the internal kernel API, it saves us so much trouble. Like in
this case, James can just add a callback without caring about any
(almost :)) of the current users.

> Those functions should take the contents of that
> struct as direct arguments.

I think we should move in the opposite direction. You are right, it
isn't neccessarily pretty, but having encapsulations like this saves
us a lot of trouble while interacting with so many other people and
extending API's all the time. It's a trade, and it's a good one, if
you need to maintain code that has so many callers, and  so many
architectures, you can't even check that you don't break them.

> I haven't finished the patch series to implement
> this, but since I noticed your patch I thought I'd better speak up now.  Here's
> the first... the idea is to eventually deprecate sysfs_[create|remove]_group()
> altogether.

Again, I don't think, that we want to get rid of the struct container
housing all the parameters and beeing open for future extensions
without changing all the callers.

>     The current declaration of struct attribute_group prevents long lists of
>     attributes from being marked const.  Ideally, the second argument to the
>     sysfs_create_group() and sysfs_remove_group() functions would be marked "deep"
>     const, but C has no such construct.  This patch provides a parallel set of
>     functions with the desired decoration.

What do we get out of this constification compared to the current code?

Thanks,
Kay
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/