Skip to contentSkip to conversation

kdbus: checking for IS_ERR() instead of NULL

PATCH In review Dan Carpenter 2 participants 3 messages Apr 22, 2015 Kernel janitors

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 kdbus: checking for IS_ERR() instead of NULL

"e->user" is never assigned an ERR_PTR(). It can be set to NULL so I assume that is what is intended here.

ipc/kdbus/queue.c +1 -1
index a449464..21fce62 100644
@@ -637,7 +637,7 @@ int kdbus_queue_entry_move(struct kdbus_queue_entry *e,
 	lockdep_assert_held(&src->lock);
 	lockdep_assert_held(&dst->lock);
 
-	if (WARN_ON(IS_ERR(e->user)) || WARN_ON(list_empty(&e->entry)))
+	if (WARN_ON(!e->user) || WARN_ON(list_empty(&e->entry)))
 		return -EINVAL;
 	if (src = dst)
 		return 0;
#1 lore raw

Hi

Quoted 10 lines · Dan Carpenter

"e->user" is never assigned an ERR_PTR(). It can be set to NULL so I assume that is what is intended here.

ipc/kdbus/queue.c +0 -0
index a449464..21fce62 100644

ipc/kdbus/queue.c int kdbus_queue_entry_move(struct kdbus_queue_entry *e, · line 640

Quoted 5 lines
ipc/kdbus/queue.c +1 -1
@@ -637,7 +637,7 @@ int kdbus_queue_entry_move(struct kdbus_queue_entry *e,
        lockdep_assert_held(&src->lock);
        lockdep_assert_held(&dst->lock);
 
-       if (WARN_ON(IS_ERR(e->user)) || WARN_ON(list_empty(&e->entry)))
+       if (WARN_ON(!e->user) || WARN_ON(list_empty(&e->entry)))

e->user is allowed to be NULL (eg., for kernel notifications). We used to set it to ERR_PTR(foobar), but that's long gone. This WARN_ON() can simply be dropped.

I wrote a patch [1] and will send it to Greg later.

Thanks
David

[1] http://cgit.freedesktop.org/~dvdhrm/linux/commit/?h=kdbus&idë132dd85d4e882a9b3ad07497b26d1b55cab893

ipc/kdbus/queue.c int kdbus_queue_entry_move(struct kdbus_queue_entry *e, · line 643

Quoted 3 lines
ipc/kdbus/queue.c +0 -0
                return -EINVAL;
        if (src = dst)
                return 0;

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#2 lore raw

Sorry, I should have just reported this instead of introducing a bug. Thanks for fixing it properly.

regards,
dan carpenter