On Tue, 10 May 2011, Jim Cromie wrote:
On Sun, May 8, 2011 at 3:23 AM, Robert P. J. Day <rpjday@crashcourse.ca> wrote:
more just a curiosity than anything else, but i'm perusing the kobject sample programs in the kernel source directory and in kobject-example.c, there's one thing about which i'm a bit puzzled.
the kernel header file sysfs.h defines the basic attribute structure as:
struct attribute { const char *name; mode_t mode; #ifdef CONFIG_DEBUG_LOCK_ALLOC struct lock_class_key *key; struct lock_class_key skey; #endif };
in other words, simply the attribute name and mode. no problem.
however, the header file kobject.h defines a more informative kobj_attribute structure thusly:
struct kobj_attribute { struct attribute attr; ssize_t (*show)(struct kobject *kobj, struct kobj_attribute *attr, char *buf); ssize_t (*store)(struct kobject *kobj, struct kobj_attribute *attr, const char *buf, size_t count); };
as you can see, it's just a regular sysfs attribute, bundled with the respective show and store routines.
what puzzles me is why those show() and store() routines are defined as being passed the larger kobj_attribute structure, rather than the simpler sysfs attribute structure. it doesn't seem as if there's much value in having the show() and store() callbacks getting those two extra pieces of information.
well, theyre passing pointers, so "larger" doesnt matter. the larger structure just has more info available. Now whether pointers to show & store are particularly useful from inside a show / store routine is debatable, as you noted. A quick grep didnt show me anything, but my eye isnt as discerning as Id like.
and that was, of course, what i was curious about -- whether there was any compelling reason for those show()/store() routines to be passed the address of the larger, enclosing structure rather than the more basic, common "struct attribute". it's not a big deal, i'm just curious as to the decision-making process that went into that.
in fact, in the sample program kobject-example.c, in the more general example, the callback routine needs to check the name of the attribute to know how to process it properly so it has to use "attr->attr.name" to access the field of the simpler "struct attribute" anyway.
thats to be expected. looking up the name is far better than having many separate show/store routines, each doing essentially the same thing, for in0, in1, in2 ...
yes, i know, but that wasn't my point. my point was that, because the larger structure address was passed, getting to the name required following the struct attribute address. in short, it just seemed overkill to pass the address of the "larger" structure when, having been passed it, the called routine simply dereferenced to get back to the enclosed struct attribute structure.
hwmon drivers used to do that (multiple fns, defined via macros), the situation is much improved now, though I think a few still remain.
FWIW, there are SENSOR_ATTRs, which extend/subclass plain-old struct attrs (modulo capitalization, approximations), maybe that possibility is what kobj_attributes are about.
static struct sensor_device_attribute fan_min[] = { SENSOR_ATTR(fan1_min, S_IWUSR | S_IRUGO, show_fan_min, set_fan_min, 0), SENSOR_ATTR(fan2_min, S_IWUSR | S_IRUGO, show_fan_min, set_fan_min, 1), SENSOR_ATTR(fan3_min, S_IWUSR | S_IRUGO, show_fan_min, set_fan_min, 2), };
hwmon-sysfs.h - hardware monitoring chip driver sysfs defines
struct sensor_device_attribute{ struct device_attribute dev_attr; int index; }; struct sensor_device_attribute_2 { struct device_attribute dev_attr; u8 index; u8 nr; };
in device.h:
struct bus_attribute { struct attribute attr; ssize_t (*show)(struct bus_type *bus, char *buf); ssize_t (*store)(struct bus_type *bus, const char *buf, size_t count); }; struct driver_attribute { struct attribute attr; ssize_t (*show)(struct device_driver *driver, char *buf); ssize_t (*store)(struct device_driver *driver, const char *buf, size_t count); }; struct class_attribute { struct attribute attr; ssize_t (*show)(struct class *class, struct class_attribute *attr, char *buf); ssize_t (*store)(struct class *class, struct class_attribute *attr, const char *buf, size_t count); }; struct device_attribute { struct attribute attr; ssize_t (*show)(struct device *dev, struct device_attribute *attr, char *buf); ssize_t (*store)(struct device *dev, struct device_attribute *attr, const char *buf, size_t count); };
I dont fully grok it, but theres clearly a pattern here.
ok, it's quite possibly a historical holdover and, as i said earlier, it's not a critically important point to clarify, i was just curious. rday -- ======================================================================== Robert P. J. Day Ottawa, Ontario, CANADA http://crashcourse.ca Twitter: http://twitter.com/rpjday LinkedIn: http://ca.linkedin.com/in/rpjday ========================================================================