Messages in this thread Patch in this message | | | Date | Sat, 29 Aug 2020 08:44:14 +0200 | From | Mauro Carvalho Chehab <> | Subject | Re: [PATCH 08/20] media: pci: saa7164.c: Replace if(cond) BUG with BUG_ON |
| |
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
| |