Re: [PATCH] exporting capability code/name pairs (try #6)

[Greg KH <greg@kroah.com>]

On Wed, Feb 20, 2008 at 01:38:59PM +0900, Kohei KaiGai wrote:
> >> If we can have a private member in kobj_attribute, we can found the 
> content
> >> to be returned in a single step.
> >
> > Ok, again, just send me a patch that adds this functionality and we will
> > be very glad to consider it.
>
> [1/2] Add a private data field within kobj_attribute structure.
>
> This patch add a private data field, declared as void *, within 
> kobj_attribute
> structure. Anyone wants to use sysfs can store their private data to refer 
> at
> _show() and _store() method.
> It enables to share a single method function with several similar entries,
> like ones to export the list of capabilities the running kernel supported.

But your patch 2/2 doesn't use this interface, why not?

>  include/linux/kobject.h |    1 +
>  include/linux/sysfs.h   |    7 +++++++
>  2 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/kobject.h b/include/linux/kobject.h
> index caa3f41..57d5bf1 100644
> --- a/include/linux/kobject.h
> +++ b/include/linux/kobject.h
> @@ -130,6 +130,7 @@ struct kobj_attribute {
>  			char *buf);
>  	ssize_t (*store)(struct kobject *kobj, struct kobj_attribute *attr,
>  			 const char *buf, size_t count);
> +	void *data;	/* a private field */

Hm, can you really use this?

>  extern struct sysfs_ops kobj_sysfs_ops;
> diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
> index 8027104..6f40ff9 100644
> --- a/include/linux/sysfs.h
> +++ b/include/linux/sysfs.h
> @@ -50,6 +50,13 @@ struct attribute_group {
>  	.store	= _store,					\
>  }
>
> +#define __ATTR_DATA(_name,_mode,_show,_store,_data) {		\
> +	.attr = {.name = __stringify(_name), .mode = _mode },   \
> +	.show	= _show,					\
> +	.store	= _store,					\
> +	.data	= (void *)(_data),				\
> +}

I don't see how this would be any different from the original?  You are
always passed a kobject, which can be embedded in anything else.

Could you also modify the documentation and the sample code to use this
new field, showing how it is to be used, and testing that it works
properly at the same time?

thanks,

greg k-h