staging: nvec: Avoid the use of BUG_ON
PATCH 1/1 staging: nvec: Avoid the use of BUG_ON
Prevent a kernel panic by avoiding the use of the BUG_ON macro and using the WARN_ON macro instead. Checkpatch detected this issue.
--- drivers/staging/nvec/nvec.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)
drivers/staging/nvec/nvec.c +12 -2
@@ -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 ;
Signature · 1 line
2.7.0
Quoted 2 lines · Laura Garcia Liebana
Prevent a kernel panic by avoiding the use of the
BUG_ONmacro and using theWARN_ONmacro instead. Checkpatch detected this issue.
Why is nvec->state = 0; the right thing to do? What will happen when the break is taken?
julia
Quoted 43 lines
Signed-off-by Laura Garcia Liebana <[email protected]>--- drivers/staging/nvec/nvec.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)drivers/staging/nvec/nvec.c +12
-3@@ -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 [email protected]. To post to this group, send email to [email protected].
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.
Quoted 46 lines · Julia Lawall
Quoted 2 lines · Laura Garcia Liebana
Prevent a kernel panic by avoiding the use of the
BUG_ONmacro and using theWARN_ONmacro instead. Checkpatch detected this issue.Why is
nvec->state= 0; the right thing to do? What will happen when the break is taken?julia
Quoted 36 lines
Signed-off-by Laura Garcia Liebana <[email protected]>--- drivers/staging/nvec/nvec.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)drivers/staging/nvec/nvec.c +12
-3@@ -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
Quoted 7 lines
Quoted 7 lines
--
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 [email protected]. To post to this group, send email to [email protected].
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.
Quoted 65 lines · Laura Garcia
Quoted 46 lines · Julia Lawall
Quoted 2 lines · Laura Garcia Liebana
Prevent a kernel panic by avoiding the use of the
BUG_ONmacro and using theWARN_ONmacro instead. Checkpatch detected this issue.Why is
nvec->state= 0; the right thing to do? What will happen when the break is taken?julia
Quoted 36 lines
Signed-off-by Laura Garcia Liebana <[email protected]>--- drivers/staging/nvec/nvec.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)drivers/staging/nvec/nvec.c +12
-3@@ -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
Quoted 7 lines
Quoted 7 lines
--
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 [email protected]. To post to this group, send email to [email protected].
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):
Quoted 8 lines
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