diff mbox series

[-next] rfkill: remove BUG_ON() in core.c

Message ID 20221021130104.469966-1-yangyingliang@huawei.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [-next] rfkill: remove BUG_ON() in core.c | expand

Checks

Context Check Description
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 4 maintainers not CCed: kuba@kernel.org pabeni@redhat.com edumazet@google.com davem@davemloft.net
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 77 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Yang Yingliang Oct. 21, 2022, 1:01 p.m. UTC 
Replace BUG_ON() with pointer check to handle fault more gracefully.

Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
 net/rfkill/core.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

Comments

Johannes Berg Oct. 21, 2022, 1:17 p.m. UTC | #1 
On Fri, 2022-10-21 at 21:01 +0800, Yang Yingliang wrote:
> Replace BUG_ON() with pointer check to handle fault more gracefully.
> 

That's basically (static) user errors though, so at least WARN_ON or
something?

johannes
Leon Romanovsky Oct. 23, 2022, 8:12 a.m. UTC | #2 
On Fri, Oct 21, 2022 at 09:01:04PM +0800, Yang Yingliang wrote:
> Replace BUG_ON() with pointer check to handle fault more gracefully.
> 
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> ---
>  net/rfkill/core.c | 29 ++++++++++++++---------------
>  1 file changed, 14 insertions(+), 15 deletions(-)
> 
> diff --git a/net/rfkill/core.c b/net/rfkill/core.c
> index dac4fdc7488a..5fc96fa24eda 100644
> --- a/net/rfkill/core.c
> +++ b/net/rfkill/core.c
> @@ -150,9 +150,8 @@ EXPORT_SYMBOL(rfkill_get_led_trigger_name);
>  
>  void rfkill_set_led_trigger_name(struct rfkill *rfkill, const char *name)
>  {
> -	BUG_ON(!rfkill);
> -
> -	rfkill->ledtrigname = name;
> +	if (rfkill)

In all these places, rfkill shouldn't be NULL from the beginning. By
adding these if (rfkill), you are saying to reviewers and code authors
that it is correct thing to do something like this
rfkill_set_led_trigger_name(NULL, "new_name"), which is of course not
true.

Thanks
diff mbox series

Patch

diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index dac4fdc7488a..5fc96fa24eda 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -150,9 +150,8 @@  EXPORT_SYMBOL(rfkill_get_led_trigger_name);
 
 void rfkill_set_led_trigger_name(struct rfkill *rfkill, const char *name)
 {
-	BUG_ON(!rfkill);
-
-	rfkill->ledtrigname = name;
+	if (rfkill)
+		rfkill->ledtrigname = name;
 }
 EXPORT_SYMBOL(rfkill_set_led_trigger_name);
 
@@ -532,7 +531,8 @@  bool rfkill_set_hw_state_reason(struct rfkill *rfkill,
 	unsigned long flags;
 	bool ret, prev;
 
-	BUG_ON(!rfkill);
+	if (!rfkill)
+		return blocked;
 
 	if (WARN(reason &
 	    ~(RFKILL_HARD_BLOCK_SIGNAL | RFKILL_HARD_BLOCK_NOT_OWNER),
@@ -581,7 +581,8 @@  bool rfkill_set_sw_state(struct rfkill *rfkill, bool blocked)
 	unsigned long flags;
 	bool prev, hwblock;
 
-	BUG_ON(!rfkill);
+	if (!rfkill)
+		return blocked;
 
 	spin_lock_irqsave(&rfkill->lock, flags);
 	prev = !!(rfkill->state & RFKILL_BLOCK_SW);
@@ -607,8 +608,8 @@  void rfkill_init_sw_state(struct rfkill *rfkill, bool blocked)
 {
 	unsigned long flags;
 
-	BUG_ON(!rfkill);
-	BUG_ON(rfkill->registered);
+	if (!rfkill || rfkill->registered)
+		return;
 
 	spin_lock_irqsave(&rfkill->lock, flags);
 	__rfkill_set_sw_state(rfkill, blocked);
@@ -622,7 +623,8 @@  void rfkill_set_states(struct rfkill *rfkill, bool sw, bool hw)
 	unsigned long flags;
 	bool swprev, hwprev;
 
-	BUG_ON(!rfkill);
+	if (!rfkill)
+		return;
 
 	spin_lock_irqsave(&rfkill->lock, flags);
 
@@ -860,9 +862,7 @@  static int rfkill_dev_uevent(struct device *dev, struct kobj_uevent_env *env)
 
 void rfkill_pause_polling(struct rfkill *rfkill)
 {
-	BUG_ON(!rfkill);
-
-	if (!rfkill->ops->poll)
+	if (!rfkill || !rfkill->ops->poll)
 		return;
 
 	rfkill->polling_paused = true;
@@ -872,9 +872,7 @@  EXPORT_SYMBOL(rfkill_pause_polling);
 
 void rfkill_resume_polling(struct rfkill *rfkill)
 {
-	BUG_ON(!rfkill);
-
-	if (!rfkill->ops->poll)
+	if (!rfkill || !rfkill->ops->poll)
 		return;
 
 	rfkill->polling_paused = false;
@@ -1115,7 +1113,8 @@  EXPORT_SYMBOL(rfkill_register);
 
 void rfkill_unregister(struct rfkill *rfkill)
 {
-	BUG_ON(!rfkill);
+	if (!rfkill)
+		return;
 
 	if (rfkill->ops->poll)
 		cancel_delayed_work_sync(&rfkill->poll_work);