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

Replace multiline CFE_ES_SYSLOG_APPEND macro with inline function #1349

Closed
skliper opened this issue Apr 14, 2021 · 1 comment · Fixed by #1368 or #1431
Closed

Replace multiline CFE_ES_SYSLOG_APPEND macro with inline function #1349

skliper opened this issue Apr 14, 2021 · 1 comment · Fixed by #1368 or #1431
Assignees
Milestone

Comments

@skliper
Copy link
Contributor

skliper commented Apr 14, 2021

Is your feature request related to a problem? Please describe.
Use of multiline macros should be avoided (per coding standard):

#define CFE_ES_SYSLOG_APPEND(LogString) \
{ \
CFE_ES_LockSharedData(__func__, __LINE__); \
CFE_ES_SysLogAppend_Unsync(LogString); \
CFE_ES_UnlockSharedData(__func__, __LINE__); \
OS_printf("%s", LogString); \
}

Describe the solution you'd like
Replaced with inline, or one line macro/inline combo

Describe alternatives you've considered
Wrap with do {...} while(0), see https://gcc.gnu.org/onlinedocs/cpp/Swallowing-the-Semicolon.html

Additional context
Code review

Requester Info
Jacob Hageman - NASA/GSFC

@jphickey
Copy link
Contributor

Reviewing how CFE_ES_SYSLOG_APPEND is currently used, I'm having a hard time justifying why it even needs to exist at all anymore. I think previously it was used in more places, perhaps, but now its down to just 4 instances in CDS code, and I can't see any reason why those 4 places can't just use CFE_ES_WriteToSysLog like everything else does.

It's an internal macro in cfe_es_log.h - not public - so my recommendation is to just remove it entirely.

jphickey added a commit to jphickey/cFE that referenced this issue Apr 16, 2021
Replace remaining uses of this internal macro with the
CFE_ES_WriteToSysLog API.
jphickey added a commit to jphickey/cFE that referenced this issue Apr 20, 2021
Replace remaining uses of this internal macro with the
CFE_ES_WriteToSysLog API.
jphickey added a commit to jphickey/cFE that referenced this issue Apr 20, 2021
Replace remaining uses of this internal macro with the
CFE_ES_WriteToSysLog API.
astrogeco added a commit that referenced this issue Apr 28, 2021
Fix #1349, remove unneeded CFE_ES_SYSLOG_APPEND macro
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants