* [PATCH 08/20] media: pci: saa7164.c: Replace if(cond) BUG with BUG_ON
@ 2020-08-07 8:35 Daniel W. S. Almeida
2020-08-29 6:44 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 2+ messages in thread
From: Daniel W. S. Almeida @ 2020-08-07 8:35 UTC (permalink / raw)
To: linux-kernel
Cc: skhan, Daniel W. S. Almeida, Mauro Carvalho Chehab, linux-media
From: "Daniel W. S. Almeida" <dwlsalmeida@gmail.com>
Fix the following coccinelle reports:
drivers/media/pci/saa7164/saa7164-buffer.c:254:3-6: WARNING: Use BUG_ON
instead of if condition followed by BUG.
drivers/media/pci/saa7164/saa7164-buffer.c:261:3-6: WARNING: Use BUG_ON
instead of if condition followed by BUG.
Found using - Coccinelle (http://coccinelle.lip6.fr)
Signed-off-by: Daniel W. S. Almeida <dwlsalmeida@gmail.com>
---
drivers/media/pci/saa7164/saa7164-buffer.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/media/pci/saa7164/saa7164-buffer.c b/drivers/media/pci/saa7164/saa7164-buffer.c
index 289cb901985b..245d9db280aa 100644
--- a/drivers/media/pci/saa7164/saa7164-buffer.c
+++ b/drivers/media/pci/saa7164/saa7164-buffer.c
@@ -250,15 +250,14 @@ int saa7164_buffer_cfg_port(struct saa7164_port *port)
list_for_each_safe(c, n, &port->dmaqueue.list) {
buf = list_entry(c, struct saa7164_buffer, list);
- if (buf->flags != SAA7164_BUFFER_FREE)
- BUG();
+ BUG_ON(buf->flags != SAA7164_BUFFER_FREE);
/* Place the buffer in the h/w queue */
saa7164_buffer_activate(buf, i);
/* Don't exceed the device maximum # bufs */
- if (i++ > port->hwcfg.buffercount)
- BUG();
+ BUG_ON(i > port->hwcfg.buffercount);
+ i++;
}
mutex_unlock(&port->dmaqueue_lock);
@@ -302,4 +301,3 @@ void saa7164_buffer_dealloc_user(struct saa7164_user_buffer *buf)
kfree(buf);
}
-
--
2.28.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH 08/20] media: pci: saa7164.c: Replace if(cond) BUG with BUG_ON
2020-08-07 8:35 [PATCH 08/20] media: pci: saa7164.c: Replace if(cond) BUG with BUG_ON Daniel W. S. Almeida
@ 2020-08-29 6:44 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 2+ messages in thread
From: Mauro Carvalho Chehab @ 2020-08-29 6:44 UTC (permalink / raw)
To: Daniel W. S. Almeida; +Cc: linux-kernel, skhan, linux-media
Hi Daniel,
Em Fri, 7 Aug 2020 05:35:35 -0300
"Daniel W. S. Almeida" <dwlsalmeida@gmail.com> escreveu:
> From: "Daniel W. S. Almeida" <dwlsalmeida@gmail.com>
>
> Fix the following coccinelle reports:
>
> drivers/media/pci/saa7164/saa7164-buffer.c:254:3-6: WARNING: Use BUG_ON
> instead of if condition followed by BUG.
>
> drivers/media/pci/saa7164/saa7164-buffer.c:261:3-6: WARNING: Use BUG_ON
> instead of if condition followed by BUG.
>
> Found using - Coccinelle (http://coccinelle.lip6.fr)
I ended accepting those patches, but IMO, this doesn't make
the code any better.
The thing is that very few parts of the Kernel should panic if
things go sideways. I don't think that any media driver would
apply on such cases.
As pointed at:
https://www.kernel.org/doc/html/v5.7/process/deprecated.html?highlight=fallthrough#implicit-switch-case-fall-through
The real fix is to use WARN_ON(), probably with something like:
if (WARN_ON(something))
return;
as probably the code after BUG() would be assuming that the
condition was asserted.
Regards,
Mauro
>
> Signed-off-by: Daniel W. S. Almeida <dwlsalmeida@gmail.com>
> ---
> drivers/media/pci/saa7164/saa7164-buffer.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/pci/saa7164/saa7164-buffer.c b/drivers/media/pci/saa7164/saa7164-buffer.c
> index 289cb901985b..245d9db280aa 100644
> --- a/drivers/media/pci/saa7164/saa7164-buffer.c
> +++ b/drivers/media/pci/saa7164/saa7164-buffer.c
> @@ -250,15 +250,14 @@ int saa7164_buffer_cfg_port(struct saa7164_port *port)
> list_for_each_safe(c, n, &port->dmaqueue.list) {
> buf = list_entry(c, struct saa7164_buffer, list);
>
> - if (buf->flags != SAA7164_BUFFER_FREE)
> - BUG();
> + BUG_ON(buf->flags != SAA7164_BUFFER_FREE);
>
> /* Place the buffer in the h/w queue */
> saa7164_buffer_activate(buf, i);
>
> /* Don't exceed the device maximum # bufs */
> - if (i++ > port->hwcfg.buffercount)
> - BUG();
> + BUG_ON(i > port->hwcfg.buffercount);
> + i++;
>
> }
> mutex_unlock(&port->dmaqueue_lock);
> @@ -302,4 +301,3 @@ void saa7164_buffer_dealloc_user(struct saa7164_user_buffer *buf)
>
> kfree(buf);
> }
> -
Thanks,
Mauro
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-08-29 6:44 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-07 8:35 [PATCH 08/20] media: pci: saa7164.c: Replace if(cond) BUG with BUG_ON Daniel W. S. Almeida
2020-08-29 6:44 ` Mauro Carvalho Chehab
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).