Re: [PATCH] exporting capability code/name pairs (try #6)
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)
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)
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/