Messages in this thread | | | Date | Tue, 15 Jan 2008 08:17:04 +0800 | From | "Dave Young" <> | Subject | Re: [PATCH 1/7] driver-core : add class iteration api |
| |
On Jan 14, 2008 8:13 PM, Cornelia Huck <cornelia.huck@de.ibm.com> wrote: > On Sat, 12 Jan 2008 17:47:54 +0800, > Dave Young <hidave.darkstar@gmail.com> wrote: > > Minor style suggestion (same for class_find_child): > > > +struct device *class_find_device(struct class *class, void *data, > > + int (*match)(struct device *, void *)) > > +{ > > + struct device *dev; > > + int error = 1; > > How about using inverse logic here (e.g., start with int found = 0)...
Sounds good, will do. Thanks.
> > > + > > + if (!class) > > + return NULL; > > + > > + down(&class->sem); > > + list_for_each_entry(dev, &class->devices, node) { > > + dev = get_device(dev); > > + if (dev) { > > + if (match(dev, data)) { > > + error = 0; > > ...and set found = 1 here... > > > + break; > > + } else > > + put_device(dev); > > + } else > > + break; > > + } > > + up(&class->sem); > > + > > + if (error) > > + return NULL; > > + return dev; > > ...and do > return found ? dev : NULL; > in the end? > > Especially since not finding the device is not really an error. > > > +} > > Otherwise this looks fine to me. >
| |