Hi,
>>>> 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.
Yoshifuji is 100% correct, IMNSHO.
Please, this is _basic_ refcounting semantics. For those who disagree,
kindly read Yoshifuji's above paragraph again.
Well, I'm okay either way. It's not like one way is undisputably better
than the other
Yes, it is, of course. Allowing xxx_put(NULL) to succeed (without any
warnings/oops) is *absolutely* nonsensical, and can *only* occur if the
caller (or worse, the API itself) is buggy in the first place (i.e. does not
use proper locking and/or refcounting).
I can't believe it should be so difficult to understand this. How can any
caller (that first did a xxx_get() on that shared object) land up with that
object getting NULL _from under it_ unless some logic is wrong
somewhere? And instead of flagging this broken logic, the proposed
change here would hide it.
Worse, if that object did become NULL between the _get() and _put()
code, then we'll have an oops (which would be even more difficult to
debug now) anyway.
but we're leaning toward accepting NULL argument in this
type of functions. Think about kfree(NULL) and its usefulness.
Don't {mis}quote the kfree() mistake here, please.
More
importantly, the ecosystem around sysfs - that is, kobject, driver model
- generally accepts NULL argument for their get/put functions
This can only mean two things:
(1) Either, they simply do not _need_ the refcounting in the first place
(which means -- better do away with get() and put() for them altogether)
(2) Or, all that code / APIs are so horribly misdesigned and/or buggy that
you're now having to hide that by allowing NULL arguments in get() and
put() functions (which means -- fix the bugs, please)
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.
Ok, what's the compelling reason to change sysfs_put() then?
I don't see any, either.
Satyam
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
