Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MISRA C Do not use recursions #11425

Closed
ceolin opened this issue Nov 16, 2018 · 6 comments
Closed

MISRA C Do not use recursions #11425

ceolin opened this issue Nov 16, 2018 · 6 comments
Assignees
Labels
area: MISRA-C bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug
Milestone

Comments

@ceolin
Copy link
Member

ceolin commented Nov 16, 2018

MISRA-C does not allows that a function call themselves, either directly or indirectly. Currently there is only one spot in Zephyr doing it _rb_walk().

rule 17.2

part of: #9552

@ceolin ceolin added Feature A planned feature with a milestone priority: medium Medium impact/importance bug area: MISRA-C labels Nov 16, 2018
@ceolin
Copy link
Member Author

ceolin commented Nov 16, 2018

@andyross if you have time can you take a look on this ?

@andyross
Copy link
Contributor

MISRA exists just to make my life miserable, doesn't it?

(FWIW: I know of at least one other in mempool, though that one is proper tail recursion and maybe MISRA has an exception for that?)

Yeah, I'll take it. FWIW: there is a non-recursive walker (a FOREACH kind of thing), but it's got significantly higher code size.

@andyross andyross self-assigned this Nov 16, 2018
@ceolin
Copy link
Member Author

ceolin commented Nov 16, 2018

it does feel to be in my shoes :)

Yeah, I'm sorry for that, I know that not using recursion here will increase and make it more complicated.

@pfalcon
Copy link
Contributor

pfalcon commented Nov 16, 2018

MISRA exists just to make my life miserable, doesn't it?

Everyone's.

proper tail recursion and maybe MISRA has an exception for that?

I'm sure tail recursion doesn't exist in MISRA's world. And speaking seriously, C doesn't guarantee tail-recursive optimization, so... just don't tell anyone there's another func like that ;-).

@nashif nashif added bug The issue is a bug, or the PR is fixing a bug and removed Feature A planned feature with a milestone labels Feb 21, 2019
@andrewboie andrewboie added this to the v1.15.0 milestone Feb 27, 2019
@andyross
Copy link
Contributor

Actually looking at this more carefully, this should be doable for 1.14. The two spots I know of are rb_walk above, and a tail recursion case in mempool. The latter can be turned into iteration with 2-3 lines of code and an extra level of indentation, and it turns out the former is in fact no longer used in the tree beyond the test case (I needed it in the scheduler at one point, but I don't think that usage ever even merged). Will submit a patch.

andyross pushed a commit to andyross/zephyr that referenced this issue Feb 27, 2019
MISRA rules (see zephyrproject-rtos#11425) forbid recursive algorithms.  In the case of
rb_walk(), it's not actually used anywhere but a test right now, so we
can simply disable the API when CONFIG_MISRA_SANE is defined.  Mempool
had a (IMHO, fairly clever) tail recursive loop in bfree_recombine()
which can be trivially transformed into an only slightly uglier
iterative version.

Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
andrewboie pushed a commit that referenced this issue Feb 28, 2019
MISRA rules (see #11425) forbid recursive algorithms.  In the case of
rb_walk(), it's not actually used anywhere but a test right now, so we
can simply disable the API when CONFIG_MISRA_SANE is defined.  Mempool
had a (IMHO, fairly clever) tail recursive loop in bfree_recombine()
which can be trivially transformed into an only slightly uglier
iterative version.

Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
@andyross
Copy link
Contributor

andyross commented Mar 1, 2019

Merged the recursion patch. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: MISRA-C bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug
Projects
None yet
Development

No branches or pull requests

5 participants