candle-usb / candleLight_fw

gs_usb compatible firmware for candleLight, cantact and canable
Other
659 stars 290 forks source link

Possible null frames added to frame pool #15

Closed willtoth closed 5 years ago

willtoth commented 5 years ago

In the case that the frame pool is empty but an rx request in pending, the logic will re-insert a null frame into the frame pool.

Specifically in main, when can_is_rx_pending returns true, but queue_pop_front is called on an empty queue and returns a null frame:

https://github.com/HubertD/candleLight_fw/blob/53b4b915af166575d7c93d63811001ae89e45e33/src/main.c#L118-L137

HubertD commented 5 years ago

you are absolutely right, this makes no sense.

jhofstee commented 5 years ago

I have a patch for this...

HubertD commented 5 years ago

i'd expect i roughly looks like this? https://github.com/candle-usb/candleLight_fw/commit/ba1e4cce659291b5de0d55f604ea694b6b1bc738?diff=unified#diff-803c5170888b8642f2a97e5e9423d399L132-L134

jhofstee commented 5 years ago

no I did:

             if (frame != 0) {
                 if (can_receive(&hCAN, frame)) {
                             received_count++;

                     frame->timestamp_us = timer_get();
                     frame->echo_id = 0xFFFFFFFF; // not a echo frame
                     frame->channel = 0;
                     frame->flags = 0;
                     frame->reserved = 0;

                     send_to_host_or_enqueue(frame);

                     led_indicate_trx(&hLED, led_1);

                 } else {
                     queue_push_back(q_frame_pool, frame);
                 }
             }

So if alloc works but can receive fails for whatever reason, it is freed again.

p.s. And I really would like to get consistent whitespace, but that is for later.

On 10/29/19 7:58 PM, Hubert Denkmair wrote:

i'd expect i roughly looks like this? ba1e4cc?diff=unified#diff-803c5170888b8642f2a97e5e9423d399L132-L134 https://github.com/candle-usb/candleLight_fw/commit/ba1e4cce659291b5de0d55f604ea694b6b1bc738?diff=unified#diff-803c5170888b8642f2a97e5e9423d399L132-L134

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/candle-usb/candleLight_fw/issues/15?email_source=notifications&email_token=AAKBDBJXAY4JSETOMC7A2ELQRCBVHA5CNFSM4HJP2ZT2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECRWIYI#issuecomment-547578977, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKBDBMH5GPNYWWA2IK3QJDQRCBVHANCNFSM4HJP2ZTQ.

HubertD commented 5 years ago

should not mess around in old code after 8 hours debugging at work, i guess. Fixed it the way you did, @jhofstee also fixed the tabs/whitespace issues in main.c