On Fri, Aug 23, 2024 at 01:26:40PM +0800, zhangjiao2 wrote:
> From: Zhang Jiao <zhangjiao2@xxxxxxxxxxxxxxxxxxxx>
>
> Use IS_ERR_OR_NULL() to make the code cleaner.
ITYM "obfuscate the bogus code".
Take a look at kthread_run() definition:
#define kthread_run(threadfn, data, namefmt, ...) \
({ \
struct task_struct *__k \
= kthread_create(threadfn, data, namefmt, ## __VA_ARGS__); \
if (!IS_ERR(__k)) \
wake_up_process(__k); \
__k; \
})
OK, what would need to happen for that to return NULL? kthread_create()
returning NULL *AND* wake_up_process(NULL) surviving, right?
int wake_up_process(struct task_struct *p)
{
return try_to_wake_up(p, TASK_NORMAL, 0);
}
OK, so we'd need try_to_wake_up(NULL, ...) to survive execution:
int try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)
{
guard(preempt)();
int cpu, success = 0;
if (p == current) {
whatever, current is never NULL or a lot of places would be
utterly screwed
}
/* some comment */
scoped_guard (raw_spinlock_irqsave, &p->pi_lock) {
... and that would start with trying to grab &NULL->pi_lock, which is not
going to survive.
> task = kthread_run(kthread, k, "%s", k->name);
> - if (task == NULL || IS_ERR(task))
> + if (IS_ERR_OR_NULL(task))
> return -ENOMEM;
In other words, task == NULL had been pointless all along. Your change only
makes it harder to spot.
IS_ERR_OR_NULL is almost never the right thing to do; there are cases where
a function may legitimately return a pointer to object, NULL *or* ERR_PTR(something),
but most of the time it's either impossible (and the caller couldn't have been
arsed to check what the calling conventions are) or a sign of a function in
bad need of saner calling conventions.
In this case it's the former - kthread_run() never returns a NULL and
actually if you look into kthread_create() you'll see that it returns
a pointer to new task_struct instance on success and ERR_PTR(-E...) on
failure. NULL is never returned by that thing.
This kind of "defensive" programming only confuses the readers; please,
don't paper over that garbage.
