Re: [PATCH 2.6.12-rc4 3/3] (dynamic sysfs callbacks) device_attribute

[Yani Ioannou <yani.ioannou@gmail.com>]

On 5/11/05, Greg KH <greg@kroah.com> wrote:
> On Wed, May 11, 2005 at 03:58:35AM -0400, Yani Ioannou wrote:
> > -static ssize_t show_in(struct device *dev, char *buf, int nr)
> > +static ssize_t show_in(struct device *dev, char *buf, void *private)
> >  {
> > +     int nr = *((int*)private);
> 
> What's wrong with a simple:
>         int nr = (int)private;

Ouch, yes thanks for catching that, that's horribly wrong. Its a
leftover from a previous example where I was actually was passing int*
not int. I'll fix up the example and resend it. That is what comes
from not being able to test it I guess.

> Sorry, but I need a real patch in email form so I can apply it.  I can
> handle a 300K+ email :)
> 
> Or you can break it up into smaller pieces, like one per major part of
> the kernel.  That is the preferred way.

I'd like to break it up, but I think even broken up by major part of
the kernel it one piece will still be too large since the majority of
the changes take place in drivers & drivers/i2c and are very
asymmetric :-(. I'll send you the patch inline privately for now.

> We should make a __ATTR macro instead, right?

Well another __ATTR macro (e.g. ATTR_PRIVATE) would make declaring the
new DEVICE_ATTR_PRIVATE macro, etc, easier. We can't really use __ATTR
nicely though when declaring static attributes and wanting to set the
private data, hence why I think there is the need for a macro (see
http://archives.andrew.net.au/lm-sensors/msg31399.html).

The question really is, is it better to just add that new parameter to
the DEVICE_ATTR macro, or to declare a new DEVICE_ATTR_PRIVATE macro
instead. The former obviously breaks a lot of code although my scripts
can generate another large patch for that too...

Thanks,
Yani