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

[Kohei KaiGai <kaigai@ak.jp.nec.com>]

Greg KH wrote:
> 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?

Really?
The following two _show() methods shared by every capabilities refer
the private member of kobj_attribute.

| +static ssize_t capability_name_show(struct kobject *kobj,
| +                    struct kobj_attribute *attr,
| +                    char *buffer)
| +{
| +    /* It returns numerical representation of capability. */
| +    return scnprintf(buffer, PAGE_SIZE, "%d\n", (int) attr->data);
| +}
| +
| +static ssize_t capability_code_show(struct kobject *kobj,
| +                    struct kobj_attribute *attr,
| +                    char *buffer)
| +{
| +    /* It returns symbolic representation of capability. */
| +    return scnprintf(buffer, PAGE_SIZE, "%s\n", (char *) attr->data);
| +}

>>  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?

Yes,

>>  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.

The intension of the latest patch is same as the version which uses
capability_attribute structure.
It enables to store the content to be returned in the expanded field.
Applying kobj_attribute killed needs to declare my own structure.

However, every entries had its own _show() method, generated by macros
automatically, in the previous version. It fundamentally differ from
the latest one.

> 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?

OK, Please wait for a while.

Thanks,
-- 
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>