Hi James:
* James Bottomley <[EMAIL PROTECTED]> [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...
> Index: BUILD-2.6/fs/sysfs/group.c
> ===================================================================
> --- BUILD-2.6.orig/fs/sysfs/group.c 2007-10-28 17:27:04.000000000 -0500
> +++ BUILD-2.6/fs/sysfs/group.c 2007-10-30 12:35:47.000000000 -0500
> @@ -16,25 +16,31 @@
> #include "sysfs.h"
>
>
> -static void remove_files(struct sysfs_dirent *dir_sd,
> +static void remove_files(struct sysfs_dirent *dir_sd, struct kobject *kobj,
> const struct attribute_group *grp)
> {
> struct attribute *const* attr;
> + int i;
>
> - for (attr = grp->attrs; *attr; attr++)
> - sysfs_hash_and_remove(dir_sd, (*attr)->name);
> + for (i = 0, attr = grp->attrs; *attr; i++, attr++)
> + if (grp->is_visible &&
> + grp->is_visible(kobj, *attr, i))
> + sysfs_hash_and_remove(dir_sd, (*attr)->name);
> }
>
> -static int create_files(struct sysfs_dirent *dir_sd,
> +static int create_files(struct sysfs_dirent *dir_sd, struct kobject *kobj,
> const struct attribute_group *grp)
> {
> struct attribute *const* attr;
> - int error = 0;
> + int error = 0, i;
>
> - for (attr = grp->attrs; *attr && !error; attr++)
> - error = sysfs_add_file(dir_sd, *attr, SYSFS_KOBJ_ATTR);
> + for (i = 0, attr = grp->attrs; *attr && !error; i++, attr++)
> + if (grp->is_visible &&
> + grp->is_visible(kobj, *attr, i))
> + error |=
> + sysfs_add_file(dir_sd, *attr, SYSFS_KOBJ_ATTR);
> if (error)
> - remove_files(dir_sd, grp);
> + remove_files(dir_sd, kobj, grp);
> return error;
> }
>
> @@ -54,7 +60,7 @@ int sysfs_create_group(struct kobject *
> } else
> sd = kobj->sd;
> sysfs_get(sd);
> - error = create_files(sd, grp);
> + error = create_files(sd, kobj, grp);
> if (error) {
> if (grp->name)
> sysfs_remove_subdir(sd);
> @@ -75,7 +81,7 @@ void sysfs_remove_group(struct kobject *
> } else
> sd = sysfs_get(dir_sd);
>
> - remove_files(sd, grp);
> + remove_files(sd, kobj, grp);
> if (grp->name)
> sysfs_remove_subdir(sd);
>
> Index: BUILD-2.6/include/linux/sysfs.h
> ===================================================================
> --- BUILD-2.6.orig/include/linux/sysfs.h 2007-10-28 17:20:06.000000000
> -0500
> +++ BUILD-2.6/include/linux/sysfs.h 2007-10-30 13:24:06.000000000 -0500
> @@ -32,6 +32,8 @@ struct attribute {
>
> struct attribute_group {
> const char *name;
> + int (*is_visible)(struct kobject *,
> + struct attribute *, int);
> struct attribute **attrs;
> };
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. Those functions should take the contents of that
struct as direct arguments. 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.
commit 5b5bc08ae31519b7012d7fc23ff73e0c6474de53
Author: Mark M. Hoffman <[EMAIL PROTECTED]>
Date: Sun Oct 21 11:49:57 2007 -0400
sysfs: allow attribute (group) lists to be const
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.
Signed-off-by: Mark M. Hoffman <[EMAIL PROTECTED]>
diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c
index d197237..b217a8e 100644
--- a/fs/sysfs/group.c
+++ b/fs/sysfs/group.c
@@ -17,71 +17,84 @@
static void remove_files(struct sysfs_dirent *dir_sd,
- const struct attribute_group *grp)
+ const struct attribute * const * attrs)
{
- struct attribute *const* attr;
+ const struct attribute *const* attr;
- for (attr = grp->attrs; *attr; attr++)
+ for (attr = attrs; *attr; attr++)
sysfs_hash_and_remove(dir_sd, (*attr)->name);
}
static int create_files(struct sysfs_dirent *dir_sd,
- const struct attribute_group *grp)
+ const struct attribute * const * attrs)
{
- struct attribute *const* attr;
+ const struct attribute *const* attr;
int error = 0;
- for (attr = grp->attrs; *attr && !error; attr++)
+ for (attr = attrs; *attr && !error; attr++)
error = sysfs_add_file(dir_sd, *attr, SYSFS_KOBJ_ATTR);
if (error)
- remove_files(dir_sd, grp);
+ remove_files(dir_sd, attrs);
return error;
}
-
-int sysfs_create_group(struct kobject * kobj,
- const struct attribute_group * grp)
+int sysfs_create_attr_group(struct kobject * kobj,
+ const char *name, const struct attribute * const * attrs)
{
struct sysfs_dirent *sd;
int error;
BUG_ON(!kobj || !kobj->sd);
- if (grp->name) {
- error = sysfs_create_subdir(kobj, grp->name, &sd);
+ if (name) {
+ error = sysfs_create_subdir(kobj, name, &sd);
if (error)
return error;
} else
sd = kobj->sd;
sysfs_get(sd);
- error = create_files(sd, grp);
- if (error) {
- if (grp->name)
- sysfs_remove_subdir(sd);
- }
+ error = create_files(sd, attrs);
+ if (error && name)
+ sysfs_remove_subdir(sd);
+
sysfs_put(sd);
return error;
}
-void sysfs_remove_group(struct kobject * kobj,
- const struct attribute_group * grp)
+int sysfs_create_group (struct kobject * kobj,
+ const struct attribute_group *grp)
+{
+ return sysfs_create_attr_group(kobj, grp->name,
+ (const struct attribute * const *)grp->attrs);
+}
+
+void sysfs_remove_attr_group(struct kobject * kobj,
+ const char *name, const struct attribute * const * attrs)
{
struct sysfs_dirent *dir_sd = kobj->sd;
struct sysfs_dirent *sd;
- if (grp->name) {
- sd = sysfs_get_dirent(dir_sd, grp->name);
+ if (name) {
+ sd = sysfs_get_dirent(dir_sd, name);
BUG_ON(!sd);
} else
sd = sysfs_get(dir_sd);
- remove_files(sd, grp);
- if (grp->name)
+ remove_files(sd, attrs);
+ if (name)
sysfs_remove_subdir(sd);
sysfs_put(sd);
}
+void sysfs_remove_group(struct kobject *kobj,
+ const struct attribute_group *grp)
+{
+ return sysfs_remove_attr_group(kobj, grp->name,
+ (const struct attribute * const *)grp->attrs);
+}
+EXPORT_SYMBOL_GPL(sysfs_create_attr_group);
+EXPORT_SYMBOL_GPL(sysfs_remove_attr_group);
EXPORT_SYMBOL_GPL(sysfs_create_group);
EXPORT_SYMBOL_GPL(sysfs_remove_group);
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index 149ab62..5066d50 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -101,10 +101,18 @@ int __must_check sysfs_create_link(struct kobject *kobj,
struct kobject *target,
const char *name);
void sysfs_remove_link(struct kobject *kobj, const char *name);
+int __must_check sysfs_create_attr_group(struct kobject *,
+ const char * name, const struct attribute * const *
attrs);
+
int __must_check sysfs_create_group(struct kobject *kobj,
- const struct attribute_group *grp);
+ const struct attribute_group *grp);
+
+void sysfs_remove_attr_group(struct kobject *,
+ const char * name, const struct attribute * const *
attrs);
+
void sysfs_remove_group(struct kobject *kobj,
- const struct attribute_group *grp);
+ const struct attribute_group *grp);
+
int sysfs_add_file_to_group(struct kobject *kobj,
const struct attribute *attr, const char *group);
void sysfs_remove_file_from_group(struct kobject *kobj,
@@ -190,8 +198,19 @@ static inline int sysfs_create_group(struct kobject *kobj,
return 0;
}
-static inline void sysfs_remove_group(struct kobject *kobj,
- const struct attribute_group *grp)
+static inline int sysfs_create_attr_group(struct kobject *k,
+ const char * name, const struct attribute * const *
attrs)
+{
+ return 0;
+}
+
+static inline void sysfs_remove_group(struct kobject * k, const struct
attribute_group * g)
+{
+ ;
+}
+
+static inline void sysfs_remove_attr_group(struct kobject * k,
+ const char * name, const struct attribute * const *
attrs)
{
;
}
Regards,
--
Mark M. Hoffman
[EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
