Re: [linuxtv-media:devel 497/499] drivers/media/platform/s5p-mfc/s5p_mfc.c:454:2-5: WARNING: Use BUG_ON

Re: [linuxtv-media:devel 497/499] drivers/media/platform/s5p-mfc/s5p_mfc.c:454:2-5: WARNING: Use BUG_ON

[Julia Lawall]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1443 bytes --]

The bug_on one doesn't look like a good idea, but the returnvar one would
make the code a little simpler.

julia

On Thu, 28 Aug 2014, kbuild test robot wrote:

> TO: Mauro Carvalho Chehab <m.chehab@samsung.com>
> CC: linux-media@vger.kernel.org
>
> Hi Mauro,
>
> First bad commit (maybe != root cause):
>
> tree:   git://linuxtv.org/media_tree.git devel
> head:   38a0731165250a0a77eff7b90ea3156d44cc7d66
> commit: 7155043c2d027c9c848c3d09badb5af2894ed652 [497/499] [media] enable COMPILE_TEST for media drivers
> :::::: branch date: 19 hours ago
> :::::: commit date: 19 hours ago
>
> >> drivers/media/platform/s5p-mfc/s5p_mfc.c:454:2-5: WARNING: Use BUG_ON
> >> drivers/media/platform/s5p-mfc/s5p_mfc.c:333:3-6: WARNING: Use BUG_ON
> >> drivers/media/platform/s5p-mfc/s5p_mfc.c:406:2-5: WARNING: Use BUG_ON
> >> drivers/media/platform/s5p-mfc/s5p_mfc.c:548:3-6: WARNING: Use BUG_ON
> >> drivers/media/platform/s5p-mfc/s5p_mfc.c:556:3-6: WARNING: Use BUG_ON
> >> drivers/media/platform/s5p-mfc/s5p_mfc.c:509:2-5: WARNING: Use BUG_ON
> >> drivers/media/platform/s5p-mfc/s5p_mfc.c:634:4-7: WARNING: Use BUG_ON
> --
> >> drivers/media/platform/davinci/vpfe_capture.c:946:5-8: Unneeded variable: "ret". Return "0" on line 951
>
> Please consider folding the attached diff :-)
>
> ---
> 0-DAY kernel build testing backend              Open Source Technology Center
> http://lists.01.org/mailman/listinfo/kbuild                 Intel Corporation
>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: TEXT/X-DIFF; NAME="bugon:s5p_mfc.patch", Size: 3947 bytes --]

From: Fengguang Wu <fengguang.wu@intel.com>
Subject: [PATCH] fix coccinelle warnings
TO: Mauro Carvalho Chehab <m.chehab@samsung.com>
CC: Mauro Carvalho Chehab <m.chehab@samsung.com>
CC: linux-arm-kernel@lists.infradead.org 
CC: linux-media@vger.kernel.org 
CC: linux-kernel@vger.kernel.org 
CC: devicetree@vger.kernel.org 

drivers/media/platform/s5p-mfc/s5p_mfc.c:454:2-5: WARNING: Use BUG_ON
drivers/media/platform/s5p-mfc/s5p_mfc.c:333:3-6: WARNING: Use BUG_ON
drivers/media/platform/s5p-mfc/s5p_mfc.c:406:2-5: WARNING: Use BUG_ON
drivers/media/platform/s5p-mfc/s5p_mfc.c:548:3-6: WARNING: Use BUG_ON
drivers/media/platform/s5p-mfc/s5p_mfc.c:556:3-6: WARNING: Use BUG_ON
drivers/media/platform/s5p-mfc/s5p_mfc.c:509:2-5: WARNING: Use BUG_ON
drivers/media/platform/s5p-mfc/s5p_mfc.c:634:4-7: WARNING: Use BUG_ON

 Use BUG_ON instead of a if condition followed by BUG.

Semantic patch information:
 This makes an effort to find cases where BUG() follows an if
 condition on an expression and replaces the if condition and BUG()
 with a BUG_ON having the conditional expression of the if statement
 as argument.

Generated by: scripts/coccinelle/misc/bugon.cocci

CC: Mauro Carvalho Chehab <m.chehab@samsung.com>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---

Please take the patch only if it's a positive warning. Thanks!

 s5p_mfc.c |   21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -329,8 +329,7 @@ static void s5p_mfc_handle_frame(struct
 		ctx->state = MFCINST_RES_CHANGE_INIT;
 		s5p_mfc_hw_call(dev->mfc_ops, clear_int_flags, dev);
 		wake_up_ctx(ctx, reason, err);
-		if (test_and_clear_bit(0, &dev->hw_lock) == 0)
-			BUG();
+		BUG_ON(test_and_clear_bit(0, &dev->hw_lock) == 0);
 		s5p_mfc_clock_off();
 		s5p_mfc_hw_call(dev->mfc_ops, try_run, dev);
 		return;
@@ -402,8 +401,7 @@ leave_handle_frame:
 		clear_work_bit(ctx);
 	s5p_mfc_hw_call(dev->mfc_ops, clear_int_flags, dev);
 	wake_up_ctx(ctx, reason, err);
-	if (test_and_clear_bit(0, &dev->hw_lock) == 0)
-		BUG();
+	BUG_ON(test_and_clear_bit(0, &dev->hw_lock) == 0);
 	s5p_mfc_clock_off();
 	/* if suspending, wake up device and do not try_run again*/
 	if (test_bit(0, &dev->enter_suspend))
@@ -450,8 +448,7 @@ static void s5p_mfc_handle_error(struct
 			break;
 		}
 	}
-	if (test_and_clear_bit(0, &dev->hw_lock) == 0)
-		BUG();
+	BUG_ON(test_and_clear_bit(0, &dev->hw_lock) == 0);
 	s5p_mfc_hw_call(dev->mfc_ops, clear_int_flags, dev);
 	s5p_mfc_clock_off();
 	wake_up_dev(dev, reason, err);
@@ -505,8 +502,7 @@ static void s5p_mfc_handle_seq_done(stru
 	}
 	s5p_mfc_hw_call(dev->mfc_ops, clear_int_flags, dev);
 	clear_work_bit(ctx);
-	if (test_and_clear_bit(0, &dev->hw_lock) == 0)
-		BUG();
+	BUG_ON(test_and_clear_bit(0, &dev->hw_lock) == 0);
 	s5p_mfc_clock_off();
 	s5p_mfc_hw_call(dev->mfc_ops, try_run, dev);
 	wake_up_ctx(ctx, reason, err);
@@ -544,16 +540,14 @@ static void s5p_mfc_handle_init_buffers(
 		} else {
 			ctx->dpb_flush_flag = 0;
 		}
-		if (test_and_clear_bit(0, &dev->hw_lock) == 0)
-			BUG();
+		BUG_ON(test_and_clear_bit(0, &dev->hw_lock) == 0);
 
 		s5p_mfc_clock_off();
 
 		wake_up(&ctx->queue);
 		s5p_mfc_hw_call(dev->mfc_ops, try_run, dev);
 	} else {
-		if (test_and_clear_bit(0, &dev->hw_lock) == 0)
-			BUG();
+		BUG_ON(test_and_clear_bit(0, &dev->hw_lock) == 0);
 
 		s5p_mfc_clock_off();
 
@@ -630,8 +624,7 @@ static irqreturn_t s5p_mfc_irq(int irq,
 				mfc_err("post_frame_start() failed\n");
 			s5p_mfc_hw_call(dev->mfc_ops, clear_int_flags, dev);
 			wake_up_ctx(ctx, reason, err);
-			if (test_and_clear_bit(0, &dev->hw_lock) == 0)
-				BUG();
+			BUG_ON(test_and_clear_bit(0, &dev->hw_lock) == 0);
 			s5p_mfc_clock_off();
 			s5p_mfc_hw_call(dev->mfc_ops, try_run, dev);
 		} else {

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: Type: TEXT/X-DIFF; NAME="returnvar:vpfe_capture.patch", Size: 1311 bytes --]

From: Fengguang Wu <fengguang.wu@intel.com>
Subject: [PATCH] fix coccinelle warnings
TO: Mauro Carvalho Chehab <m.chehab@samsung.com>
CC: Mauro Carvalho Chehab <m.chehab@samsung.com>
CC: linux-media@vger.kernel.org 
CC: davinci-linux-open-source@linux.davincidsp.com 
CC: linux-kernel@vger.kernel.org 

drivers/media/platform/davinci/vpfe_capture.c:946:5-8: Unneeded variable: "ret". Return "0" on line 951


 Removes unneeded variable used to store return value.

Generated by: scripts/coccinelle/misc/returnvar.cocci

CC: Mauro Carvalho Chehab <m.chehab@samsung.com>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---

Please take the patch only if it's a positive warning. Thanks!

 vpfe_capture.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

--- a/drivers/media/platform/davinci/vpfe_capture.c
+++ b/drivers/media/platform/davinci/vpfe_capture.c
@@ -943,12 +943,11 @@ static int vpfe_g_fmt_vid_cap(struct fil
 				struct v4l2_format *fmt)
 {
 	struct vpfe_device *vpfe_dev = video_drvdata(file);
-	int ret = 0;
 
 	v4l2_dbg(1, debug, &vpfe_dev->v4l2_dev, "vpfe_g_fmt_vid_cap\n");
 	/* Fill in the information about format */
 	*fmt = vpfe_dev->fmt;
-	return ret;
+	return 0;
 }
 
 static int vpfe_enum_fmt_vid_cap(struct file *file, void  *priv,

Re: [linuxtv-media:devel 497/499] drivers/media/platform/s5p-mfc/s5p_mfc.c:454:2-5: WARNING: Use BUG_ON

[Fengguang Wu]

Hi Mauro,

On Wed, Aug 27, 2014 at 02:31:37PM -0300, Mauro Carvalho Chehab wrote:
> Hi Fengguang,
> 
> Not sure why, but I didn't receive this e-mail (or other emails in this
> thread).

It's a manual check process rather than an email problem.  The TO/CC
you see lie in the email body. The original email is sent to the
kbuild list for manual checking. We do such check for all low
confident static check warnings. Julia and Dan are actively looking at
these reports and will forward the ones that worth more attention to
the relevant people.

Thanks,
Fengguang

> Em Wed, 27 Aug 2014 19:07:45 +0200 (CEST)
> Julia Lawall <julia.lawall@lip6.fr> escreveu:
> 
> > The bug_on one doesn't look like a good idea, but the returnvar one would
> > make the code a little simpler.
> > 
> > julia
> > 
> > On Thu, 28 Aug 2014, kbuild test robot wrote:
> > 
> > > TO: Mauro Carvalho Chehab <m.chehab@samsung.com>
> > > CC: linux-media@vger.kernel.org
> > >
> > > Hi Mauro,
> > >
> > > First bad commit (maybe != root cause):
> > >
> > > tree:   git://linuxtv.org/media_tree.git devel
> > > head:   38a0731165250a0a77eff7b90ea3156d44cc7d66
> > > commit: 7155043c2d027c9c848c3d09badb5af2894ed652 [497/499] [media] enable COMPILE_TEST for media drivers
> > > :::::: branch date: 19 hours ago
> > > :::::: commit date: 19 hours ago
> > >
> > > >> drivers/media/platform/s5p-mfc/s5p_mfc.c:454:2-5: WARNING: Use BUG_ON
> > > >> drivers/media/platform/s5p-mfc/s5p_mfc.c:333:3-6: WARNING: Use BUG_ON
> > > >> drivers/media/platform/s5p-mfc/s5p_mfc.c:406:2-5: WARNING: Use BUG_ON
> > > >> drivers/media/platform/s5p-mfc/s5p_mfc.c:548:3-6: WARNING: Use BUG_ON
> > > >> drivers/media/platform/s5p-mfc/s5p_mfc.c:556:3-6: WARNING: Use BUG_ON
> > > >> drivers/media/platform/s5p-mfc/s5p_mfc.c:509:2-5: WARNING: Use BUG_ON
> > > >> drivers/media/platform/s5p-mfc/s5p_mfc.c:634:4-7: WARNING: Use BUG_ON
> > > --
> > > >> drivers/media/platform/davinci/vpfe_capture.c:946:5-8: Unneeded variable: "ret". Return "0" on line 951
> > >
> > > Please consider folding the attached diff :-)
> > >
> > > ---
> > > 0-DAY kernel build testing backend              Open Source Technology Center
> > > http://lists.01.org/mailman/listinfo/kbuild                 Intel Corporation
> > >