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

2008-02-19 Thread Greg KH
On Wed, Feb 20, 2008 at 02:38:16PM +0900, Kohei KaiGai wrote:
> 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);
> | +}

Ah, sorry, missed that.  I also missed where this was set up as well :(

thanks,

greg k-h
--
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/


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

2008-02-19 Thread Kohei KaiGai

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 <[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/


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

2008-02-19 Thread Greg KH
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
--
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/