All of lore.kernel.org
 help / color / mirror / Atom feed
From: jim.cromie@gmail.com (Jim Cromie)
To: lm-sensors@vger.kernel.org
Subject: [lm-sensors] [patch] hwmon: fix unchecked returncodes in w83791d
Date: Mon, 25 Sep 2006 18:08:16 +0000	[thread overview]
Message-ID: <45181B10.2070807@gmail.com> (raw)
In-Reply-To: <20060925192533.40929faf.khali@linux-fr.org>

Jean Delvare wrote:
>> Fix unchecked return-status by replacing all unchecked calls 
>> to device_create_file with a single group declaration,
>> and one call to sysfs_create_group, and check that one return status.
>>
>> Signed-off-by:  Jim Cromie  <jim.cromie at gmail.com>
>> ---
>> $ diffstat pc-set/hwmon-unchecked-return-status-fixes-w83791d.patch
>>  w83791d.c |   83 +++++++++++++++++++++++++++++++++++++++++---------------------
>>  1 files changed, 55 insertions(+), 28 deletions(-)
>>
>> ---
>>
>> Its impressive (or embarrassing) how much slop you detect.
>> I trust this comment is in right place for auto-strip to 
>> keep it out of Changelog.  Is any of this automated, and if so,
>> are we/I getting it right (or close) ?
>>     
>
> Yes, anything between "---" and the beginning of the actual patch is
> dropped upstream. But I'm still editing things manually anyway so it's
> no big deal where you add your comments.
>
>   
>> @@ -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.

> Thanks,
>   

-------------- next part --------------
A non-text attachment was scrubbed...
Name: hwmon-unchecked-return-status-fixes-w83791d.patch
Type: text/x-patch
Size: 3343 bytes
Desc: not available
Url : http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20060925/773e9a77/attachment.bin 

  reply	other threads:[~2006-09-25 18:08 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-09-25 17:25 [lm-sensors] [patch] hwmon: fix unchecked returncodes in w83791d Jean Delvare
2006-09-25 18:08 ` Jim Cromie [this message]
2006-09-25 21:01 ` Jean Delvare
  -- strict thread matches above, loose matches on Subject: below --
2006-09-25 17:05 Jim Cromie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=45181B10.2070807@gmail.com \
    --to=jim.cromie@gmail.com \
    --cc=lm-sensors@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.