* [bug report] can: peak: add support for PEAK PCAN-PCIe FD CAN-FD boards
@ 2017-04-26 19:22 Dan Carpenter
2017-05-02 11:07 ` Stephane Grosjean
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2017-04-26 19:22 UTC (permalink / raw)
To: s.grosjean; +Cc: linux-can
Hello Stephane Grosjean,
The patch 8ac8321e4a79: "can: peak: add support for PEAK PCAN-PCIe FD
CAN-FD boards" from Jan 19, 2017, leads to the following static
checker warning:
drivers/net/can/peak_canfd/peak_canfd.c:402 pucan_handle_status()
error: uninitialized symbol 'cf'.
drivers/net/can/peak_canfd/peak_canfd.c
362 skb = alloc_can_err_skb(ndev, &cf);
^^
If skb is NULL "cf" is uninitialized.
363
364 /* test state error bits according to their priority */
365 if (pucan_status_is_busoff(msg)) {
366 netdev_dbg(ndev, "Bus-off entry status\n");
367 priv->can.state = CAN_STATE_BUS_OFF;
368 priv->can.can_stats.bus_off++;
369 can_bus_off(ndev);
370 if (skb)
371 cf->can_id |= CAN_ERR_BUSOFF;
372
373 } else if (pucan_status_is_passive(msg)) {
374 netdev_dbg(ndev, "Error passive status\n");
375 priv->can.state = CAN_STATE_ERROR_PASSIVE;
376 priv->can.can_stats.error_passive++;
377 if (skb) {
378 cf->can_id |= CAN_ERR_CRTL;
379 cf->data[1] = (priv->bec.txerr > priv->bec.rxerr) ?
380 CAN_ERR_CRTL_TX_PASSIVE :
381 CAN_ERR_CRTL_RX_PASSIVE;
382 cf->data[6] = priv->bec.txerr;
383 cf->data[7] = priv->bec.rxerr;
384 }
385
386 } else if (pucan_status_is_warning(msg)) {
387 netdev_dbg(ndev, "Error warning status\n");
388 priv->can.state = CAN_STATE_ERROR_WARNING;
389 priv->can.can_stats.error_warning++;
390 if (skb) {
391 cf->can_id |= CAN_ERR_CRTL;
392 cf->data[1] = (priv->bec.txerr > priv->bec.rxerr) ?
393 CAN_ERR_CRTL_TX_WARNING :
394 CAN_ERR_CRTL_RX_WARNING;
395 cf->data[6] = priv->bec.txerr;
396 cf->data[7] = priv->bec.rxerr;
397 }
398
399 } else if (priv->can.state != CAN_STATE_ERROR_ACTIVE) {
400 /* back to ERROR_ACTIVE */
401 netdev_dbg(ndev, "Error active status\n");
402 can_change_state(ndev, cf, CAN_STATE_ERROR_ACTIVE,
403 CAN_STATE_ERROR_ACTIVE);
Presumably we want an "if (skb) " here like we have on the other paths.
404 } else {
405 dev_kfree_skb(skb);
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* [bug report] can: peak: add support for PEAK PCAN-PCIe FD CAN-FD boards
@ 2017-04-26 19:24 Dan Carpenter
0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2017-04-26 19:24 UTC (permalink / raw)
To: s.grosjean; +Cc: linux-can
Hello Stephane Grosjean,
The patch 8ac8321e4a79: "can: peak: add support for PEAK PCAN-PCIe FD
CAN-FD boards" from Jan 19, 2017, leads to the following static
checker warning:
drivers/net/can/peak_canfd/peak_canfd.c:504 peak_canfd_handle_msgs_list()
error: uninitialized symbol 'msg_size'.
drivers/net/can/peak_canfd/peak_canfd.c
487 /* handle a list of rx_count messages from rx_msg memory address */
488 int peak_canfd_handle_msgs_list(struct peak_canfd_priv *priv,
489 struct pucan_rx_msg *msg_list, int msg_count)
490 {
491 void *msg_ptr = msg_list;
492 int i, msg_size;
493
494 for (i = 0; i < msg_count; i++) {
^^^^^^^^^^^^^
Can msg_count be zero?
495 msg_size = peak_canfd_handle_msg(priv, msg_ptr);
496
497 /* a null packet can be found at the end of a list */
498 if (msg_size <= 0)
499 break;
500
501 msg_ptr += msg_size;
502 }
503
504 if (msg_size < 0)
^^^^^^^^^^^^
If so, then that's a problem.
505 return msg_size;
506
507 return i;
508 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [bug report] can: peak: add support for PEAK PCAN-PCIe FD CAN-FD boards
2017-04-26 19:22 [bug report] can: peak: add support for PEAK PCAN-PCIe FD CAN-FD boards Dan Carpenter
@ 2017-05-02 11:07 ` Stephane Grosjean
2017-05-02 11:29 ` Oliver Hartkopp
0 siblings, 1 reply; 4+ messages in thread
From: Stephane Grosjean @ 2017-05-02 11:07 UTC (permalink / raw)
To: linux-can; +Cc: Dan Carpenter
Hello all,
How to proceed to include the fix?
- do I resend a complete v5 of the serie?
- do I post a single patch for this fix only?
Stephane
Le 26/04/2017 à 21:22, Dan Carpenter a écrit :
> Hello Stephane Grosjean,
>
> The patch 8ac8321e4a79: "can: peak: add support for PEAK PCAN-PCIe FD
> CAN-FD boards" from Jan 19, 2017, leads to the following static
> checker warning:
>
> drivers/net/can/peak_canfd/peak_canfd.c:402 pucan_handle_status()
> error: uninitialized symbol 'cf'.
>
> drivers/net/can/peak_canfd/peak_canfd.c
> 362 skb = alloc_can_err_skb(ndev, &cf);
> ^^
> If skb is NULL "cf" is uninitialized.
>
> 363
> 364 /* test state error bits according to their priority */
> 365 if (pucan_status_is_busoff(msg)) {
> 366 netdev_dbg(ndev, "Bus-off entry status\n");
> 367 priv->can.state = CAN_STATE_BUS_OFF;
> 368 priv->can.can_stats.bus_off++;
> 369 can_bus_off(ndev);
> 370 if (skb)
> 371 cf->can_id |= CAN_ERR_BUSOFF;
> 372
> 373 } else if (pucan_status_is_passive(msg)) {
> 374 netdev_dbg(ndev, "Error passive status\n");
> 375 priv->can.state = CAN_STATE_ERROR_PASSIVE;
> 376 priv->can.can_stats.error_passive++;
> 377 if (skb) {
> 378 cf->can_id |= CAN_ERR_CRTL;
> 379 cf->data[1] = (priv->bec.txerr > priv->bec.rxerr) ?
> 380 CAN_ERR_CRTL_TX_PASSIVE :
> 381 CAN_ERR_CRTL_RX_PASSIVE;
> 382 cf->data[6] = priv->bec.txerr;
> 383 cf->data[7] = priv->bec.rxerr;
> 384 }
> 385
> 386 } else if (pucan_status_is_warning(msg)) {
> 387 netdev_dbg(ndev, "Error warning status\n");
> 388 priv->can.state = CAN_STATE_ERROR_WARNING;
> 389 priv->can.can_stats.error_warning++;
> 390 if (skb) {
> 391 cf->can_id |= CAN_ERR_CRTL;
> 392 cf->data[1] = (priv->bec.txerr > priv->bec.rxerr) ?
> 393 CAN_ERR_CRTL_TX_WARNING :
> 394 CAN_ERR_CRTL_RX_WARNING;
> 395 cf->data[6] = priv->bec.txerr;
> 396 cf->data[7] = priv->bec.rxerr;
> 397 }
> 398
> 399 } else if (priv->can.state != CAN_STATE_ERROR_ACTIVE) {
> 400 /* back to ERROR_ACTIVE */
> 401 netdev_dbg(ndev, "Error active status\n");
> 402 can_change_state(ndev, cf, CAN_STATE_ERROR_ACTIVE,
> 403 CAN_STATE_ERROR_ACTIVE);
>
> Presumably we want an "if (skb) " here like we have on the other paths.
>
> 404 } else {
> 405 dev_kfree_skb(skb);
>
>
> regards,
> dan carpenter
> --
> To unsubscribe from this list: send the line "unsubscribe linux-can" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Stéphane Grosjean
PEAK-System France
132, rue André Bisiaux
F54320 MAXEVILLE
Tél : +(33) 9.72.54.51.97
--
PEAK-System Technik GmbH
Sitz der Gesellschaft Darmstadt - HRB 9183
Geschaeftsfuehrung: Alexander Gach / Uwe Wilhelm
--
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [bug report] can: peak: add support for PEAK PCAN-PCIe FD CAN-FD boards
2017-05-02 11:07 ` Stephane Grosjean
@ 2017-05-02 11:29 ` Oliver Hartkopp
0 siblings, 0 replies; 4+ messages in thread
From: Oliver Hartkopp @ 2017-05-02 11:29 UTC (permalink / raw)
To: Stephane Grosjean, linux-can; +Cc: Dan Carpenter
Hi Stephane,
On 05/02/2017 01:07 PM, Stephane Grosjean wrote:
> How to proceed to include the fix?
>
> - do I resend a complete v5 of the serie?
>
> - do I post a single patch for this fix only?
Yes :-)
The entire series is already part of Dave Millers net-next tree which
will go into Linus' tree the next days.
So Daves net-next is currently the best base to fix this.
Best regards,
Oliver
>
> Stephane
>
>
> Le 26/04/2017 à 21:22, Dan Carpenter a écrit :
>> Hello Stephane Grosjean,
>>
>> The patch 8ac8321e4a79: "can: peak: add support for PEAK PCAN-PCIe FD
>> CAN-FD boards" from Jan 19, 2017, leads to the following static
>> checker warning:
>>
>> drivers/net/can/peak_canfd/peak_canfd.c:402 pucan_handle_status()
>> error: uninitialized symbol 'cf'.
>>
>> drivers/net/can/peak_canfd/peak_canfd.c
>> 362 skb = alloc_can_err_skb(ndev, &cf);
>> ^^
>> If skb is NULL "cf" is uninitialized.
>>
>> 363
>> 364 /* test state error bits according to their priority */
>> 365 if (pucan_status_is_busoff(msg)) {
>> 366 netdev_dbg(ndev, "Bus-off entry status\n");
>> 367 priv->can.state = CAN_STATE_BUS_OFF;
>> 368 priv->can.can_stats.bus_off++;
>> 369 can_bus_off(ndev);
>> 370 if (skb)
>> 371 cf->can_id |= CAN_ERR_BUSOFF;
>> 372
>> 373 } else if (pucan_status_is_passive(msg)) {
>> 374 netdev_dbg(ndev, "Error passive status\n");
>> 375 priv->can.state = CAN_STATE_ERROR_PASSIVE;
>> 376 priv->can.can_stats.error_passive++;
>> 377 if (skb) {
>> 378 cf->can_id |= CAN_ERR_CRTL;
>> 379 cf->data[1] = (priv->bec.txerr >
>> priv->bec.rxerr) ?
>> 380
>> CAN_ERR_CRTL_TX_PASSIVE :
>> 381 CAN_ERR_CRTL_RX_PASSIVE;
>> 382 cf->data[6] = priv->bec.txerr;
>> 383 cf->data[7] = priv->bec.rxerr;
>> 384 }
>> 385
>> 386 } else if (pucan_status_is_warning(msg)) {
>> 387 netdev_dbg(ndev, "Error warning status\n");
>> 388 priv->can.state = CAN_STATE_ERROR_WARNING;
>> 389 priv->can.can_stats.error_warning++;
>> 390 if (skb) {
>> 391 cf->can_id |= CAN_ERR_CRTL;
>> 392 cf->data[1] = (priv->bec.txerr >
>> priv->bec.rxerr) ?
>> 393
>> CAN_ERR_CRTL_TX_WARNING :
>> 394 CAN_ERR_CRTL_RX_WARNING;
>> 395 cf->data[6] = priv->bec.txerr;
>> 396 cf->data[7] = priv->bec.rxerr;
>> 397 }
>> 398
>> 399 } else if (priv->can.state != CAN_STATE_ERROR_ACTIVE) {
>> 400 /* back to ERROR_ACTIVE */
>> 401 netdev_dbg(ndev, "Error active status\n");
>> 402 can_change_state(ndev, cf,
>> CAN_STATE_ERROR_ACTIVE,
>> 403 CAN_STATE_ERROR_ACTIVE);
>>
>> Presumably we want an "if (skb) " here like we have on the other paths.
>>
>> 404 } else {
>> 405 dev_kfree_skb(skb);
>>
>>
>> regards,
>> dan carpenter
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-can" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-05-02 11:29 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-04-26 19:22 [bug report] can: peak: add support for PEAK PCAN-PCIe FD CAN-FD boards Dan Carpenter
2017-05-02 11:07 ` Stephane Grosjean
2017-05-02 11:29 ` Oliver Hartkopp
-- strict thread matches above, loose matches on Subject: below --
2017-04-26 19:24 Dan Carpenter
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.