[lm-sensors] [patch] hwmon: fix unchecked returncodes in w83791d
Jean Delvare
khali at linux-fr.org
Mon Sep 25 23:01:19 CEST 2006
Hi Jim,
> > > @@ -1029,6 +1054,8 @@ static int w83791d_detach_client(struct
> > > if (data)
> > > hwmon_device_unregister(data->class_dev);
> > >
> > > + sysfs_remove_group(&client->dev.kobj, &w83791d_group);
> > > +
> > > if ((err = i2c_detach_client(client)))
> > > return err;
> >
> > The "if (data)" is used to differenciate between the real client and
> > the subclients, exactly as is done in the w83781d driver. As subclients
> > have no files, the call to sysfs_remove_group() should be made
> > conditional as well. If not, it'll still work, but with a significant
> > performance drop.
>
> on this item, I actually checked :-)
>
> w83791d_detect will fail unless data is sucessfully allocated,
> so it must be there if detach_client is called.
>
> but I suppose its safer (less action-at-a-distance) your way, esp wrt
> any future changes.
> redo attached.
I'm sorry to insist, but you overlooked another function which also
attaches clients: w83791d_create_subclient(). And this one does _not_
associate data to the client. All three clients (one main and two
subclients) have the same driver, so w83791d_detach_client() is called
for all of them. The "if (data)" test is there exactly for this reason,
so this test is really needed.
The new patch looks OK to me, thanks.
--
Jean Delvare
More information about the lm-sensors
mailing list