public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <htejun@gmail.com>
To: "YOSHIFUJI Hideaki / 吉藤英明" <yoshfuji@linux-ipv6.org>
Cc: gregkh@suse.de, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 24/61] sysfs: make sysfs_put() ignore NULL sd
Date: Thu, 12 Jul 2007 12:00:40 +0900	[thread overview]
Message-ID: <46959958.6080005@gmail.com> (raw)
In-Reply-To: <20070712.100617.14203407.yoshfuji@linux-ipv6.org>

YOSHIFUJI Hideaki / 吉藤英明 wrote:
> In article <20070711235529.GA7485@suse.de> (at Wed, 11 Jul 2007 16:55:29 -0700), Greg KH <gregkh@suse.de> says:
> 
>> On Thu, Jul 12, 2007 at 08:50:47AM +0900, YOSHIFUJI Hideaki / ?$B5HF#1QL@ wrote:
>>> In article <11841968512510-git-send-email-gregkh@suse.de> (at Wed, 11 Jul 2007 16:31:43 -0700), Greg Kroah-Hartman <gregkh@suse.de> says:
>>>
>>>> Make sysfs_put() ignore NULL sd instead of oopsing.
>>> I do not think this is a good idea; it is non-sense (and rather a bug)
>>> to call "put" with NULL argument in general.
>> It's better than having to check it all the time in the caller :)
> 
> How many callers do we have that will get benefit from this change?
> 
> Well, the change will hide the bug. It seems all callers in fs/sysfs
> already assume that the argument is NOT NULL, and it is a bug to call
> sysfs_put() with NULL; the function should be used to "put" something
> you "have" (non-NULL). If it is called with NULL, I would say, we
> should BUG here to detect the logical bug.

Well, I'm okay either way.  It's not like one way is undisputably better
than the other but we're leaning toward accepting NULL argument in this
type of functions.  Think about kfree(NULL) and its usefulness.  More
importantly, the ecosystem around sysfs - that is, kobject, driver model
- generally accepts NULL argument for their get/put functions, so unless
there's a compelling reason to convert them all, and I don't see any,
sysfs_put() needs to follow the same rule.

-- 
tejun

  reply	other threads:[~2007-07-12  3:00 UTC|newest]

Thread overview: 81+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-07-11 23:30 [GIT PATCH] sysfs and driver core patches for 2.6.22 Greg KH
2007-07-11 23:31 ` [PATCH 01/61] Rules on how to use sysfs in userspace programs Greg Kroah-Hartman
2007-07-11 23:31   ` [PATCH 02/61] debugfs: add rename for debugfs files Greg Kroah-Hartman
2007-07-11 23:31     ` [PATCH 03/61] DMI-based module autoloading Greg Kroah-Hartman
2007-07-11 23:31       ` [PATCH 04/61] Driver core: add missing kset uevent Greg Kroah-Hartman
2007-07-11 23:31         ` [PATCH 05/61] sysdev: use mutex instead of semaphore Greg Kroah-Hartman
2007-07-11 23:31           ` [PATCH 06/61] Power Management: use mutexes instead of semaphores Greg Kroah-Hartman
2007-07-11 23:31             ` [PATCH 07/61] PM: Remove pm_parent from struct dev_pm_info Greg Kroah-Hartman
2007-07-11 23:31               ` [PATCH 08/61] PM: Remove saved_state " Greg Kroah-Hartman
2007-07-11 23:31                 ` [PATCH 09/61] PM: Simplify suspend_device Greg Kroah-Hartman
2007-07-11 23:31                   ` [PATCH 10/61] Driver core: include linux/mutex.h from attribute_container.c Greg Kroah-Hartman
2007-07-11 23:31                     ` [PATCH 11/61] driver core: properly get driver in device_release_driver Greg Kroah-Hartman
2007-07-11 23:31                       ` [PATCH 12/61] driver core: fix kernel doc of device_release_driver Greg Kroah-Hartman
2007-07-11 23:31                         ` [PATCH 13/61] Driver core: fix devres_release_all() return value Greg Kroah-Hartman
2007-07-11 23:31                           ` [PATCH 14/61] PM: Remove prev_state from struct dev_pm_info Greg Kroah-Hartman
2007-07-11 23:31                             ` [PATCH 15/61] PM: Remove power_state.event checks from suspend core code Greg Kroah-Hartman
2007-07-11 23:31                               ` [PATCH 16/61] PM: Do not check parent state in suspend and resume " Greg Kroah-Hartman
2007-07-11 23:31                                 ` [PATCH 17/61] PM: do not use saved_state from struct dev_pm_info on ARM Greg Kroah-Hartman
2007-07-11 23:31                                   ` [PATCH 18/61] Driver core: coding style cleanup Greg Kroah-Hartman
2007-07-11 23:31                                     ` [PATCH 19/61] idr: fix obscure bug in allocation path Greg Kroah-Hartman
2007-07-11 23:31                                       ` [PATCH 20/61] idr: separate out idr_mark_full() Greg Kroah-Hartman
2007-07-11 23:31                                         ` [PATCH 21/61] ida: implement idr based id allocator Greg Kroah-Hartman
2007-07-11 23:31                                           ` [PATCH 22/61] sysfs: move release_sysfs_dirent() to dir.c Greg Kroah-Hartman
2007-07-11 23:31                                             ` [PATCH 23/61] sysfs: allocate inode number using ida Greg Kroah-Hartman
2007-07-11 23:31                                               ` [PATCH 24/61] sysfs: make sysfs_put() ignore NULL sd Greg Kroah-Hartman
2007-07-11 23:31                                                 ` [PATCH 25/61] sysfs: fix error handling in binattr write() Greg Kroah-Hartman
2007-07-11 23:31                                                   ` [PATCH 26/61] sysfs: flatten cleanup paths in sysfs_add_link() and create_dir() Greg Kroah-Hartman
2007-07-11 23:31                                                     ` [PATCH 27/61] sysfs: flatten and fix sysfs_rename_dir() error handling Greg Kroah-Hartman
2007-07-11 23:31                                                       ` [PATCH 28/61] sysfs: consolidate sysfs_dirent creation functions Greg Kroah-Hartman
2007-07-11 23:31                                                         ` [PATCH 29/61] sysfs: add sysfs_dirent->s_parent Greg Kroah-Hartman
2007-07-11 23:31                                                           ` [PATCH 30/61] sysfs: add sysfs_dirent->s_name Greg Kroah-Hartman
2007-07-11 23:31                                                             ` [PATCH 31/61] sysfs: make sysfs_dirent->s_element a union Greg Kroah-Hartman
2007-07-11 23:31                                                               ` [PATCH 32/61] sysfs: implement kobj_sysfs_assoc_lock Greg Kroah-Hartman
2007-07-11 23:31                                                                 ` [PATCH 33/61] sysfs: reimplement symlink using sysfs_dirent tree Greg Kroah-Hartman
2007-07-11 23:31                                                                   ` [PATCH 34/61] sysfs: implement bin_buffer Greg Kroah-Hartman
2007-07-11 23:31                                                                     ` [PATCH 35/61] sysfs: implement sysfs_dirent active reference and immediate disconnect Greg Kroah-Hartman
2007-07-11 23:31                                                                       ` [PATCH 36/61] sysfs: kill attribute file orphaning Greg Kroah-Hartman
2007-07-11 23:31                                                                         ` [PATCH 37/61] sysfs: separate out sysfs_attach_dentry() Greg Kroah-Hartman
2007-07-11 23:31                                                                           ` [PATCH 38/61] sysfs: reimplement sysfs_drop_dentry() Greg Kroah-Hartman
2007-07-11 23:31                                                                             ` [PATCH 39/61] sysfs: kill unnecessary attribute->owner Greg Kroah-Hartman
2007-07-11 23:31                                                                               ` [PATCH 40/61] driver-core: make devt_attr and uevent_attr static Greg Kroah-Hartman
2007-07-11 23:32                                                                                 ` [PATCH 41/61] sysfs: make sysfs_alloc_ino() static Greg Kroah-Hartman
2007-07-11 23:32                                                                                   ` [PATCH 42/61] sysfs: fix parent refcounting during rename and move Greg Kroah-Hartman
2007-07-11 23:32                                                                                     ` [PATCH 43/61] sysfs: reorganize sysfs_new_indoe() and sysfs_create() Greg Kroah-Hartman
2007-07-11 23:32                                                                                       ` [PATCH 44/61] sysfs: use iget_locked() instead of new_inode() Greg Kroah-Hartman
2007-07-11 23:32                                                                                         ` [PATCH 45/61] sysfs: fix root sysfs_dirent -> root dentry association Greg Kroah-Hartman
2007-07-11 23:32                                                                                           ` [PATCH 46/61] sysfs: move s_active functions to fs/sysfs/dir.c Greg Kroah-Hartman
2007-07-11 23:32                                                                                             ` [PATCH 47/61] sysfs: slim down sysfs_dirent->s_active Greg Kroah-Hartman
2007-07-11 23:32                                                                                               ` [PATCH 48/61] sysfs: use singly-linked list for sysfs_dirent tree Greg Kroah-Hartman
2007-07-11 23:32                                                                                                 ` [PATCH 49/61] sysfs: Fix oops in sysfs_drop_dentry on x86_64 Greg Kroah-Hartman
2007-07-11 23:32                                                                                                   ` [PATCH 50/61] sysfs: make sysfs_drop_dentry() access inodes using ilookup() Greg Kroah-Hartman
2007-07-11 23:32                                                                                                     ` [PATCH 51/61] sysfs: rename sysfs_dirent->s_type to s_flags and make room for flags Greg Kroah-Hartman
2007-07-11 23:32                                                                                                       ` [PATCH 52/61] sysfs: implement SYSFS_FLAG_REMOVED flag Greg Kroah-Hartman
2007-07-11 23:32                                                                                                         ` [PATCH 53/61] sysfs: implement sysfs_find_dirent() and sysfs_get_dirent() Greg Kroah-Hartman
2007-07-11 23:32                                                                                                           ` [PATCH 54/61] sysfs: make kobj point to sysfs_dirent instead of dentry Greg Kroah-Hartman
2007-07-11 23:32                                                                                                             ` [PATCH 55/61] sysfs: consolidate sysfs spinlocks Greg Kroah-Hartman
2007-07-11 23:32                                                                                                               ` [PATCH 56/61] sysfs: use sysfs_mutex to protect the sysfs_dirent tree Greg Kroah-Hartman
2007-07-11 23:32                                                                                                                 ` [PATCH 57/61] sysfs: restructure add/remove paths and fix inode update Greg Kroah-Hartman
2007-07-11 23:32                                                                                                                   ` [PATCH 58/61] sysfs: move sysfs_drop_dentry() to dir.c and make it static Greg Kroah-Hartman
2007-07-11 23:32                                                                                                                     ` [PATCH 59/61] sysfs: implement sysfs_get_dentry() Greg Kroah-Hartman
2007-07-11 23:32                                                                                                                       ` [PATCH 60/61] sysfs: make directory dentries and inodes reclaimable Greg Kroah-Hartman
2007-07-11 23:32                                                                                                                         ` [PATCH 61/61] sysfs: add parameter "struct bin_attribute *" in .read/.write methods for sysfs binary attributes Greg Kroah-Hartman
2007-07-11 23:50                                                 ` [PATCH 24/61] sysfs: make sysfs_put() ignore NULL sd YOSHIFUJI Hideaki / 吉藤英明
2007-07-11 23:55                                                   ` Greg KH
2007-07-12  1:06                                                     ` YOSHIFUJI Hideaki / 吉藤英明
2007-07-12  3:00                                                       ` Tejun Heo [this message]
2007-07-12 19:46                                                         ` Satyam Sharma
2007-07-13  4:21                                                           ` Tejun Heo
2007-07-13  5:03                                                             ` Tejun Heo
2007-07-13 17:28                                                               ` Satyam Sharma
2007-07-14  3:01                                                                 ` Tejun Heo
2007-07-14  4:27                                                                   ` Satyam Sharma
2007-07-14  4:52                                                                     ` Tejun Heo
2007-07-11 23:38   ` [PATCH 01/61] Rules on how to use sysfs in userspace programs Robert P. J. Day
2007-07-11 23:43     ` Greg KH
2007-07-12 10:39       ` Rene Herman
2007-07-12  8:48   ` Pavel Machek
2007-07-12 21:59     ` Kay Sievers
2007-07-12 22:14     ` Greg KH
2007-07-12 22:41       ` Pavel Machek
2007-07-13  2:14         ` Greg KH

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=46959958.6080005@gmail.com \
    --to=htejun@gmail.com \
    --cc=gregkh@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=yoshfuji@linux-ipv6.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox