Skip to contentSkip to conversation

can: kvaser_usb: Consolidate and unify state change handling

DISCUSSION Dan Carpenter 2 participants 3 messages Feb 5, 2015 CAN network drivers

lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

Hello Ahmed S. Darwish,

The patch 96d7f10634e6: "can: kvaser_usb: Consolidate and unify state change handling" from Jan 26, 2015, leads to the following Smatch warning:

	drivers/net/can/usb/kvaser_usb.c:827 kvaser_usb_rx_error_update_can_state()
	warn: impossible condition '(es->txerr >= 256) => (0-255 >= 256)'

drivers/net/can/usb/kvaser_usb.c
824 else if (es->status & M16C_STATE_BUS_PASSIVE)
825 new_state = CAN_STATE_ERROR_PASSIVE;
826 else if (es->status & M16C_STATE_BUS_ERROR) {
827 if ((es->txerr >= 256) || (es->rxerr >= 256))

   828                          new_state = CAN_STATE_BUS_OFF;
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Dead code. We normally ignore these "impossible condition" warnings if it feels like the condition makes the code more readable. But in this case, I wonder if the code works as intended. It's not clear to me.

   829                  else if ((es->txerr >= 128) || (es->rxerr >= 128))
   830                          new_state = CAN_STATE_ERROR_PASSIVE;
   831                  else if ((es->txerr >= 96) || (es->rxerr >= 96))

regards,
dan carpenter

maintainer
#1 lore raw

Hey Dan,

Quoted 6 lines · Dan Carpenter

The patch 96d7f10634e6: "can: kvaser_usb: Consolidate and unify state change handling" from Jan 26, 2015, leads to the following Smatch warning:

	drivers/net/can/usb/kvaser_usb.c:827 kvaser_usb_rx_error_update_can_state()
	warn: impossible condition '(es->txerr >= 256) => (0-255 >= 256)'

A patch that implicitly fixes this warning, along with other things, is in my latest pull request to David Miller.

    can: kvaser_usb: Ignore spurious error events after a busoff
    http://article.gmane.org/gmane.linux.can/7587
    http://www.spinics.net/lists/netdev/msg315062.html

regards,
Marc

Signature · 4 lines
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |
maintainer
#2 lore raw
Quoted 7 lines · Marc Kleine-Budde

Hey Dan,

Quoted 4 lines · Dan Carpenter

The patch 96d7f10634e6: "can: kvaser_usb: Consolidate and unify state change handling" from Jan 26, 2015, leads to the following Smatch warning:

Quoted 5 lines
Quoted 2 lines
	drivers/net/can/usb/kvaser_usb.c:827 kvaser_usb_rx_error_update_can_state()
	warn: impossible condition '(es->txerr >= 256) => (0-255 >= 256)'

A patch that implicitly fixes this warning, along with other things, is in my latest pull request to David Miller.

...which has just been merged.

Marc

Signature · 4 lines
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |