* [PATCH] staging: nvec: Avoid the use of BUG_ON
@ 2016-02-21 8:15 Laura Garcia Liebana
2016-02-21 12:31 ` [Outreachy kernel] " Julia Lawall
0 siblings, 1 reply; 4+ messages in thread
From: Laura Garcia Liebana @ 2016-02-21 8:15 UTC (permalink / raw)
To: outreachy-kernel
Prevent a kernel panic by avoiding the use of the BUG_ON macro and using the
WARN_ON macro instead. Checkpatch detected this issue.
Signed-off-by: Laura Garcia Liebana <nevola@gmail.com>
---
drivers/staging/nvec/nvec.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c
index c335ae2..3b5b94f 100644
--- a/drivers/staging/nvec/nvec.c
+++ b/drivers/staging/nvec/nvec.c
@@ -641,11 +641,21 @@ static irqreturn_t nvec_interrupt(int irq, void *dev)
nvec_msg_free(nvec, nvec->rx);
nvec->state = 3;
nvec_tx_set(nvec);
- BUG_ON(nvec->tx->size < 1);
+ if (WARN_ON(nvec->tx->size < 1)) {
+ dev_err(nvec->dev,
+ "Invalid TX size\n");
+ nvec->state = 0;
+ break;
+ }
to_send = nvec->tx->data[0];
nvec->tx->pos = 1;
} else if (status == (I2C_SL_IRQ)) {
- BUG_ON(nvec->rx == NULL);
+ if (WARN_ON(!nvec->rx)) {
+ dev_err(nvec->dev,
+ "Invalid RX\n");
+ nvec->state = 0;
+ break;
+ }
nvec->rx->data[1] = received;
nvec->rx->pos = 2;
nvec->state = 4;
--
2.7.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [Outreachy kernel] [PATCH] staging: nvec: Avoid the use of BUG_ON
2016-02-21 8:15 [PATCH] staging: nvec: Avoid the use of BUG_ON Laura Garcia Liebana
@ 2016-02-21 12:31 ` Julia Lawall
2016-02-21 16:51 ` Laura Garcia
0 siblings, 1 reply; 4+ messages in thread
From: Julia Lawall @ 2016-02-21 12:31 UTC (permalink / raw)
To: Laura Garcia Liebana; +Cc: outreachy-kernel
On Sun, 21 Feb 2016, Laura Garcia Liebana wrote:
> Prevent a kernel panic by avoiding the use of the BUG_ON macro and using the
> WARN_ON macro instead. Checkpatch detected this issue.
Why is nvec->state = 0; the right thing to do? What will happen when the
break is taken?
julia
> Signed-off-by: Laura Garcia Liebana <nevola@gmail.com>
> ---
> drivers/staging/nvec/nvec.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c
> index c335ae2..3b5b94f 100644
> --- a/drivers/staging/nvec/nvec.c
> +++ b/drivers/staging/nvec/nvec.c
> @@ -641,11 +641,21 @@ static irqreturn_t nvec_interrupt(int irq, void *dev)
> nvec_msg_free(nvec, nvec->rx);
> nvec->state = 3;
> nvec_tx_set(nvec);
> - BUG_ON(nvec->tx->size < 1);
> + if (WARN_ON(nvec->tx->size < 1)) {
> + dev_err(nvec->dev,
> + "Invalid TX size\n");
> + nvec->state = 0;
> + break;
> + }
> to_send = nvec->tx->data[0];
> nvec->tx->pos = 1;
> } else if (status == (I2C_SL_IRQ)) {
> - BUG_ON(nvec->rx == NULL);
> + if (WARN_ON(!nvec->rx)) {
> + dev_err(nvec->dev,
> + "Invalid RX\n");
> + nvec->state = 0;
> + break;
> + }
> nvec->rx->data[1] = received;
> nvec->rx->pos = 2;
> nvec->state = 4;
> --
> 2.7.0
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20160221081553.GA4327%40sonyv.
> For more options, visit https://groups.google.com/d/optout.
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Outreachy kernel] [PATCH] staging: nvec: Avoid the use of BUG_ON
2016-02-21 12:31 ` [Outreachy kernel] " Julia Lawall
@ 2016-02-21 16:51 ` Laura Garcia
2016-02-22 0:43 ` Julia Lawall
0 siblings, 1 reply; 4+ messages in thread
From: Laura Garcia @ 2016-02-21 16:51 UTC (permalink / raw)
To: Julia Lawall; +Cc: outreachy-kernel
On Sun, Feb 21, 2016 at 07:31:12AM -0500, Julia Lawall wrote:
> On Sun, 21 Feb 2016, Laura Garcia Liebana wrote:
>
> > Prevent a kernel panic by avoiding the use of the BUG_ON macro and using the
> > WARN_ON macro instead. Checkpatch detected this issue.
>
> Why is nvec->state = 0; the right thing to do? What will happen when the
> break is taken?
>
> julia
>
> > Signed-off-by: Laura Garcia Liebana <nevola@gmail.com>
> > ---
> > drivers/staging/nvec/nvec.c | 14 ++++++++++++--
> > 1 file changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c
> > index c335ae2..3b5b94f 100644
> > --- a/drivers/staging/nvec/nvec.c
> > +++ b/drivers/staging/nvec/nvec.c
> > @@ -641,11 +641,21 @@ static irqreturn_t nvec_interrupt(int irq, void *dev)
> > nvec_msg_free(nvec, nvec->rx);
> > nvec->state = 3;
> > nvec_tx_set(nvec);
> > - BUG_ON(nvec->tx->size < 1);
> > + if (WARN_ON(nvec->tx->size < 1)) {
> > + dev_err(nvec->dev,
> > + "Invalid TX size\n");
> > + nvec->state = 0;
> > + break;
> > + }
> > to_send = nvec->tx->data[0];
> > nvec->tx->pos = 1;
> > } else if (status == (I2C_SL_IRQ)) {
> > - BUG_ON(nvec->rx == NULL);
> > + if (WARN_ON(!nvec->rx)) {
> > + dev_err(nvec->dev,
> > + "Invalid RX\n");
> > + nvec->state = 0;
> > + break;
> > + }
> > nvec->rx->data[1] = received;
> > nvec->rx->pos = 2;
> > nvec->state = 4;
> > --
> > 2.7.0
> >
> > --
> > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > To post to this group, send email to outreachy-kernel@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20160221081553.GA4327%40sonyv.
> > For more options, visit https://groups.google.com/d/optout.
> >
I got inspired by an earlier case, for example in case 1:
nvec->rx = nvec_msg_alloc(nvec, NVEC_MSG_RX);
/* Should not happen in a normal world */
if (unlikely(!nvec->rx)) {
nvec->state = 0;
break;
}
A similar case appears in the case 2 (same function):
if (nvec->rx->data[0] != 0x01) {
dev_err(nvec->dev,
"Read without prior read command\n");
nvec->state = 0;
break;
}
Please let me know your thoughts about it.
Kind Regards.
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Outreachy kernel] [PATCH] staging: nvec: Avoid the use of BUG_ON
2016-02-21 16:51 ` Laura Garcia
@ 2016-02-22 0:43 ` Julia Lawall
0 siblings, 0 replies; 4+ messages in thread
From: Julia Lawall @ 2016-02-22 0:43 UTC (permalink / raw)
To: Laura Garcia; +Cc: outreachy-kernel
On Sun, 21 Feb 2016, Laura Garcia wrote:
> On Sun, Feb 21, 2016 at 07:31:12AM -0500, Julia Lawall wrote:
> > On Sun, 21 Feb 2016, Laura Garcia Liebana wrote:
> >
> > > Prevent a kernel panic by avoiding the use of the BUG_ON macro and using the
> > > WARN_ON macro instead. Checkpatch detected this issue.
> >
> > Why is nvec->state = 0; the right thing to do? What will happen when the
> > break is taken?
> >
> > julia
> >
> > > Signed-off-by: Laura Garcia Liebana <nevola@gmail.com>
> > > ---
> > > drivers/staging/nvec/nvec.c | 14 ++++++++++++--
> > > 1 file changed, 12 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c
> > > index c335ae2..3b5b94f 100644
> > > --- a/drivers/staging/nvec/nvec.c
> > > +++ b/drivers/staging/nvec/nvec.c
> > > @@ -641,11 +641,21 @@ static irqreturn_t nvec_interrupt(int irq, void *dev)
> > > nvec_msg_free(nvec, nvec->rx);
> > > nvec->state = 3;
> > > nvec_tx_set(nvec);
> > > - BUG_ON(nvec->tx->size < 1);
> > > + if (WARN_ON(nvec->tx->size < 1)) {
> > > + dev_err(nvec->dev,
> > > + "Invalid TX size\n");
> > > + nvec->state = 0;
> > > + break;
> > > + }
> > > to_send = nvec->tx->data[0];
> > > nvec->tx->pos = 1;
> > > } else if (status == (I2C_SL_IRQ)) {
> > > - BUG_ON(nvec->rx == NULL);
> > > + if (WARN_ON(!nvec->rx)) {
> > > + dev_err(nvec->dev,
> > > + "Invalid RX\n");
> > > + nvec->state = 0;
> > > + break;
> > > + }
> > > nvec->rx->data[1] = received;
> > > nvec->rx->pos = 2;
> > > nvec->state = 4;
> > > --
> > > 2.7.0
> > >
> > > --
> > > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> > > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > > To post to this group, send email to outreachy-kernel@googlegroups.com.
> > > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20160221081553.GA4327%40sonyv.
> > > For more options, visit https://groups.google.com/d/optout.
> > >
> I got inspired by an earlier case, for example in case 1:
>
> nvec->rx = nvec_msg_alloc(nvec, NVEC_MSG_RX);
> /* Should not happen in a normal world */
> if (unlikely(!nvec->rx)) {
> nvec->state = 0;
> break;
> }
>
> A similar case appears in the case 2 (same function):
>
> if (nvec->rx->data[0] != 0x01) {
> dev_err(nvec->dev,
> "Read without prior read command\n");
> nvec->state = 0;
> break;
> }
>
> Please let me know your thoughts about it.
I don't know. It seems risky. I think that one would have to consult
with the developers. I don't know if these particular problems can be
survived.
julia
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-02-22 0:43 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-21 8:15 [PATCH] staging: nvec: Avoid the use of BUG_ON Laura Garcia Liebana
2016-02-21 12:31 ` [Outreachy kernel] " Julia Lawall
2016-02-21 16:51 ` Laura Garcia
2016-02-22 0:43 ` Julia Lawall
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.