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

Kohei KaiGai
2008-02-20T05:39:26Z

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.