lkml.org 
[lkml]   [2007]   [Oct]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] sysfs: add filter function to groups
On Oct 31, 2007 1:40 AM, Mark M. Hoffman <mhoffman@lightlink.com> wrote:
> * James Bottomley <James.Bottomley@SteelEye.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@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2007-10-31 03:05    [W:0.155 / U:1.320 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site