Chen Gang <gang.c...@asianux.com> writes: > On 2013年04月10日 14:57, Rusty Russell wrote: >> Chen Gang <gang.c...@asianux.com> writes: >>> > for EXPORT_SYMBOL_GPL functions, necessary to check their parameters. >>> > >>> > Signed-off-by: Chen Gang <gang.c...@asianux.com> >> Why? >> >> If someone misuses these functions, they crash and thus indicate that >> the caller shouldn't do that. >> > > for me, I think: > > if it is used by self (such as static functions): > I prefer to crash immediatly. > it will help us to find issue, quickly. > > if it can be used by others (such as EXPORT_SYMBOL_GPL): > I prefer to return fail and tell caller that parameter is invalid. > it is more polite to callers, and still indicate it may be an issue. > > :-)
I disagree. Calling with invalid parameters is a bug. You've just covered up some cases of invalid use and made it less likely to be found. Because the caller won't notice they screwed up. We could sprinkle WARN_ON() everywhere, but I prefer the crash. Even harder to ignore. There's no limit to how many of these checks we could put in, and we can *never* take them out. I don't want to code that way. >> Or is someone already doing this? >> > > really has: > > kernel: __wake_up_sync_key in kernel/sched/core.c. > lib: *printf. > mm: kfree. No, I mean "is someone calling these functions with NULL". Cheers, Rusty. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/