Skip to content

drivers/cc2420: don't append to FIFO if iol->iol_len == 0#11331

Merged
miri64 merged 1 commit intoRIOT-OS:masterfrom
kYc0o:pr/cc2420/fix_0_len_pkt
Apr 2, 2019
Merged

drivers/cc2420: don't append to FIFO if iol->iol_len == 0#11331
miri64 merged 1 commit intoRIOT-OS:masterfrom
kYc0o:pr/cc2420/fix_0_len_pkt

Conversation

@kYc0o
Copy link
Copy Markdown
Contributor

@kYc0o kYc0o commented Apr 2, 2019

Contribution description

Fix assertion failure as described on #11163.

Testing procedure

See #11163 .

Issues/PRs references

#11163

This prevents SPI transfers with len == 0, which triggers
an assert failure.
@kYc0o kYc0o added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Apr 2, 2019
@kYc0o kYc0o added this to the Release 2019.04 milestone Apr 2, 2019
@kYc0o kYc0o requested a review from miri64 April 2, 2019 14:09
Copy link
Copy Markdown
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested with z1. Since gnrc_networking is too big for that board, I used examples/default with txtsnd instead of the proposed testing procedures and sniffed the traffic.

Without this PR I can send non-zero payloads (see pkt 1 in wireshark screenshot beloow) but with an empty payload the node runs into an assertion:

> txtsnd 4 bcast "abcd"
2019-04-02 16:21:21,481 - INFO #  txtsnd 4 bcast "abcd"
> txtsnd 4 bcast ""
2019-04-02 16:21:27,297 - INFO #  txtsnd 4 bcast ""
2019-04-02 16:21:27,316 - INFO # n/a
2019-04-02 16:21:27,316 - INFO # *** RIOT kernel panic:
2019-04-02 16:21:27,316 - INFO # FAILED ASSERTION.
2019-04-02 16:21:27,316 - INFO # 
2019-04-02 16:21:27,317 - INFO # 	pid | name                 | state    Q | pri | stack  ( used) | base addr  | current     
2019-04-02 16:21:27,324 - INFO # 	  - | isr_stack            | -        - |   - |    256 (   -1) |     0xffff |     0xffff
2019-04-02 16:21:27,332 - INFO # 	  1 | idle                 | pending  Q |  15 |     96 (   58) |     0x111a |     0x1140 
2019-04-02 16:21:27,340 - INFO # 	  2 | main                 | pending  Q |   7 |    512 (  452) |     0x117a |     0x124e 
2019-04-02 16:21:27,349 - INFO # 	  3 | pktdump              | bl rx    _ |   6 |    512 (  148) |     0x1966 |     0x1ad4 
2019-04-02 16:21:27,358 - INFO # 	  4 | cc2420               | running  Q |   2 |    512 (  408) |     0x155c |     0x15ca 
2019-04-02 16:21:27,359 - INFO # 	    | SUM                  |            |     |   1888 ( 1066)
2019-04-02 16:21:27,359 - INFO # 
2019-04-02 16:21:27,359 - INFO # *** halted.
2019-04-02 16:21:27,359 - INFO # 
2019-04-02 16:21:33,407 - INFO # Exiting Pyterm

With this PR everything I am also able to send empty payloads (see pkt 2 and 3 below):

> txtsnd 4 bcast "abcd"
2019-04-02 16:22:23,787 - INFO # txtsnd 4 bcast "abcd"
> txtsnd 4 bcast ""
2019-04-02 16:22:26,130 - INFO # txtsnd 4 bcast ""

Sniffing something weird…

So ACK.

@miri64 miri64 added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Apr 2, 2019
@miri64 miri64 merged commit 4e464bd into RIOT-OS:master Apr 2, 2019
@kYc0o
Copy link
Copy Markdown
Contributor Author

kYc0o commented Apr 2, 2019

Thanks @miri64 !

@kYc0o kYc0o deleted the pr/cc2420/fix_0_len_pkt branch April 2, 2019 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants