[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