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

Unsafe macros, investigate conversion into Inline functions #172

Closed
skliper opened this issue Sep 30, 2019 · 8 comments · Fixed by #726
Closed

Unsafe macros, investigate conversion into Inline functions #172

skliper opened this issue Sep 30, 2019 · 8 comments · Fixed by #726
Labels
Milestone

Comments

@skliper
Copy link
Contributor

skliper commented Sep 30, 2019

Some places in CFE -- notably, in ccsds.h -- there are macros
that do things like this:

{{{
#define CCSDS_WR_APID(phdr,value)
( (phdr).foo[0] = (value >> 8),
(phdr).foo[1] = (value & 0xFF) )
}}}

This means that if "value" is an expression, odd things happen
when the macro is expanded. This appears in many places in ccsds.h
and we need to do a scan of other headers as well.

It needs to be:
{{{
#define CCSDS_WR_APID(phdr,value)
( (phdr).foo[0] = ((value) >> 8),
(phdr).foo[1] = ((value) & 0xFF) )
}}}
This change needs to happen for all the places where a macro
parameter is an operand in an experession, and does not yet
have parens around it.

UPDATE 2017-10-27: CCB recommends that in many cases we would
benefit from changing these macros into C99 "inline" functions,
so this ticket is being hijacked!

@skliper skliper self-assigned this Sep 30, 2019
@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Imported from trac issue 141. Created by glimes on 2016-03-08T13:45:59, last modified: 2019-03-01T16:09:57

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by glimes on 2016-10-18 15:00:28:

redispatching these tickets to clear my name from the "owner" field,
so people willing to work on tickets will not think that I am already
working these issues ;)

@skliper skliper added the bug label Sep 30, 2019
@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by glimes on 2016-11-08 14:17:08:

Current crop of cfe-next are all going into CFE 6.6

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by glimes on 2017-10-27 14:26:50:

CCB recommends that this scrub looking for places where parentheses should be inserted should also be combined with a review for which of these macros should be C99 "inline" functions instead.

See also ticket #248 for the motivation.

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by cdknight on 2017-10-27 14:27:16:

see also ticket #245 :)

@skliper skliper removed their assignment Sep 30, 2019
@skliper skliper added this to the 6.8.0 milestone Feb 26, 2020
@skliper
Copy link
Contributor Author

skliper commented Feb 26, 2020

I think this is also a coding standard violation.

@skliper skliper changed the title Investigate conversion of Macros into Inline functions Unsafe macros, investigate conversion into Inline functions Feb 26, 2020
@skliper skliper modified the milestones: 6.8.0, 7.0.0 May 5, 2020
@skliper
Copy link
Contributor Author

skliper commented May 5, 2020

Targeting 7.0 along with the rest of the message abstraction refactoring. Better done as a coordinated change with appropriate scoping/abstraction.

@skliper
Copy link
Contributor Author

skliper commented Aug 26, 2020

Note ccsds.h was cleaned up with #726. Remaining issues split into specific tickets (did a full scrub), see #843 and #845. Closing this general ticket.

@skliper skliper closed this as completed Aug 26, 2020
@skliper skliper linked a pull request Aug 26, 2020 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants