lkml.org 
[lkml]   [2008]   [Jan]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 0/7] convert semaphore to mutex in struct class
    Dave Young wrote:
    > This is the first one of the series about driver core changes.

    Please always provide kerneldoc comments when you add new API elements;
    here: exported functions.

    It's unfortunate that the driver core's API isn't fully documented yet,
    and you shouldn't make it worse.

    That's only my personal opinion as one API user though. But others
    might agree. Among else, things worth documenting are return values
    after errors, side effects (!), constraints on the calling context if
    there are any special constraints.

    I assume you didn't write documentation yet because you need general
    feedback first.

    ...
    > +struct device *class_find_device(struct class *class, void *data,
    > + int (*match)(struct device *, void *))
    > +{
    > + struct device *dev;
    > +
    > + if (!class)
    > + return NULL;
    > +
    > + mutex_lock(&class->mutex);
    > + list_for_each_entry(dev, &class->devices, node)
    > + if (match(dev, data) && get_device(dev))
    > + break;
    > + mutex_unlock(&class->mutex);
    > +
    > + return dev;
    > +}

    What is returned if there was no match?
    What if there was a match but get_ failed?

    ...
    > +struct class_device *class_find_child(struct class *class, void *data,
    > + int (*match)(struct class_device *, void *))
    > +{
    ...
    > + mutex_lock(&class->mutex);
    > + list_for_each_entry(dev, &class->children, node)
    > + if (match(dev, data) && class_device_get(dev))
    > + break;
    > + mutex_unlock(&class->mutex);
    > +
    > + return dev;
    > +}

    Here too?
    --
    Stefan Richter
    -=====-==--- ---= -=-=-
    http://arcgraph.de/sr/

    \
     
     \ /
      Last update: 2008-01-10 13:39    [from the cache]
    ©2003-2014 Jasper Spaans. Advertise on this site