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.

Sign up for GitHub

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

Jump to bottom

fix nasa#2316 - adding CFE_TIME_StringFmt() and CFE_TIME_StringFmtLen… #2345

Closed
wants to merge 1 commit into from
Closed

fix nasa#2316 - adding CFE_TIME_StringFmt() and CFE_TIME_StringFmtLen… #2345

wants to merge 1 commit into from

Conversation

CDKnightNASA
Copy link
Contributor

…() functions

Checklist (Please check before submitting)

Describe the contribution
Fixes #2316, adds CFE_TIME_StringFmt() and CFE_TIME_StringFmtLen() functions that behave similarly to strftime (but limited to %Y [yyyy], %j [doy], %H, %M, %S, %f [micro]). CFE_TIME_Print() calls CFE_TIME_StringFmt().

Testing performed
Added UT tests, all pass.

Expected behavior changes
No impact to behavior, adds capabilities (that will be extended to EVS so that log message timestamps can be configurable.)

System(s) tested on

  • Hardware: PC
  • OS: Debian "Bookworm" (12)
  • Versions: HEAD of main branch of cFE

Additional context
An alternative would be to use strftime, if all target platforms include it. This would allow for month/day-of-month and other options this code does not include and would be difficult to implement.

Third party code
None

Contributor Info - All information REQUIRED for consideration of pull request
[email protected]

@CDKnightNASA CDKnightNASA self-assigned this May 22, 2023
@CDKnightNASA CDKnightNASA marked this pull request as draft May 22, 2023 15:46
github-advanced-security[bot]
github-advanced-security bot found potential problems May 22, 2023
View reviewed changes
modules/time/fsw/src/cfe_time_api.c Fixed Show fixed Hide fixed
modules/time/fsw/src/cfe_time_api.c Fixed Show fixed Hide fixed
modules/time/fsw/src/cfe_time_api.c Fixed Show fixed Hide fixed
modules/time/fsw/src/cfe_time_api.c Fixed Show fixed Hide fixed
modules/time/fsw/src/cfe_time_api.c Fixed Show fixed Hide fixed
modules/time/fsw/src/cfe_time_api.c Fixed Show fixed Hide fixed
modules/time/fsw/src/cfe_time_api.c Fixed Show fixed Hide fixed
modules/time/fsw/src/cfe_time_api.c Fixed Show fixed Hide fixed
@skliper
Copy link
Contributor

skliper commented May 24, 2023

Consider returning errors and remove low level CFE_ES_WriteToSysLog use so the calling function can report issues to avoid recursion if/where possible. This is another case where it would be nice to have a health parameter in tlm vs relying on syslog.

@CDKnightNASA
Copy link
Contributor Author

Consider returning errors and remove low level CFE_ES_WriteToSysLog use so the calling function can report issues to avoid recursion if/where possible. This is another case where it would be nice to have a health parameter in tlm vs relying on syslog.

Copy on the WriteToSysLog, latest revs removes. (Automated tests flagged this as well, nice!) All of the new automated checks are making me do more work. :) Will take out of draft once I've addressed all issues.

github-advanced-security[bot]
github-advanced-security bot found potential problems May 24, 2023
View reviewed changes
modules/core_api/ut-stubs/src/cfe_time_stubs.c Outdated
{
UT_GenStub_AddParam(CFE_TIME_Print, char *, PrintBuffer);
UT_GenStub_AddParam(CFE_TIME_Print, CFE_TIME_SysTime_t, TimeToPrint);

UT_GenStub_Execute(CFE_TIME_Print, Basic, UT_DefaultHandler_CFE_TIME_Print);
UT_GenStub_Execute(CFE_TIME_Print, Basic, NULL);

Check warning

Code scanning / CodeQL

Uses of recursion

The function CFE_TIME_Print is indirectly recursive via this call to [UT_Stub_RegisterContextWithMetaData](1).
modules/time/fsw/src/cfe_time_api.c Fixed Show fixed Hide fixed
modules/core_api/ut-stubs/src/cfe_time_stubs.c Fixed Show fixed Hide fixed
modules/time/fsw/src/cfe_time_api.c Fixed Show fixed Hide fixed
modules/time/fsw/src/cfe_time_api.c Fixed Show fixed Hide fixed
modules/time/fsw/src/cfe_time_api.c Fixed Show fixed Hide fixed
modules/time/fsw/src/cfe_time_api.c Fixed Show fixed Hide fixed
github-advanced-security[bot]
github-advanced-security bot found potential problems May 24, 2023
View reviewed changes
modules/core_api/ut-stubs/src/cfe_time_stubs.c
UT_GenStub_Execute(CFE_TIME_Print, Basic, UT_DefaultHandler_CFE_TIME_Print);
UT_GenStub_Execute(CFE_TIME_Print, Basic, NULL);

return UT_GenStub_GetReturnValue(CFE_TIME_Print, CFE_Status_t);

Check warning

Code scanning / CodeQL

Uses of recursion

The function CFE_TIME_Print is indirectly recursive via this call to [UT_Stub_GetReturnValuePtr](1).
@CDKnightNASA CDKnightNASA force-pushed the fix-2316-time_fmt branch 3 times, most recently from e231851 to 9c6b04c Compare May 25, 2023 15:15
github-advanced-security[bot]
github-advanced-security bot found potential problems May 25, 2023
View reviewed changes
modules/core_api/ut-stubs/src/cfe_time_stubs.c
{
UT_GenStub_SetupReturnBuffer(CFE_TIME_Print, CFE_Status_t);

Check warning

Code scanning / CodeQL

Uses of recursion

The function CFE_TIME_Print is indirectly recursive via this call to [UT_Stub_RegisterReturnType](1).
@CDKnightNASA CDKnightNASA force-pushed the fix-2316-time_fmt branch 2 times, most recently from 0190725 to 43cfeba Compare May 25, 2023 17:14
@CDKnightNASA CDKnightNASA reopened this May 25, 2023
@CDKnightNASA CDKnightNASA force-pushed the fix-2316-time_fmt branch 2 times, most recently from e3b9713 to 56dd61c Compare May 25, 2023 23:24
github-advanced-security[bot]
github-advanced-security bot found potential problems May 25, 2023
View reviewed changes
modules/core_api/ut-stubs/src/cfe_time_stubs.c
@@ -305,12 +305,16 @@
* Generated stub function for CFE_TIME_Print()
* ----------------------------------------------------
*/
void CFE_TIME_Print(char *PrintBuffer, CFE_TIME_SysTime_t TimeToPrint)
CFE_Status_t CFE_TIME_Print(char *PrintBuffer, CFE_TIME_SysTime_t TimeToPrint)

Check notice

Code scanning / CodeQL

Long function without assertion

All functions of more than 10 lines should have at least one assertion.
modules/time/fsw/src/cfe_time_api.c
@@ -563,150 +565,28 @@
* See description in header file for argument/return detail
*
*-----------------------------------------------------------------*/
void CFE_TIME_Print(char *PrintBuffer, CFE_TIME_SysTime_t TimeToPrint)
CFE_Status_t CFE_TIME_Print(char *PrintBuffer, CFE_TIME_SysTime_t TimeToPrint)

Check notice

Code scanning / CodeQL

Long function without assertion

All functions of more than 10 lines should have at least one assertion.
github-advanced-security[bot]
github-advanced-security bot found potential problems May 25, 2023
View reviewed changes
modules/time/fsw/src/cfe_time_api.c
*PrintBuffer++ = '0' + (char)(NumberOfMicros % 10);
*PrintBuffer++ = '\0';
time_t sec = TimeToPrint.Seconds;
FmtLen = strftime(PrintBuffer, CFE_TIME_PRINTED_STRING_SIZE - 6, "%Y-%j-%H:%M:%S", gmtime(&sec));

Check failure

Code scanning / CodeQL

Use of potentially dangerous function

Call to 'gmtime' is potentially dangerous.
@dmknutsen dmknutsen added this to the Equuleus milestone May 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@CDKnightNASA CDKnightNASA

Labels
Equuleus-rc1
Projects
None yet
Milestone
Equuleus
Development

Successfully merging this pull request may close these issues.

create CFE_TIME_PrintFmt();
3 participants
@CDKnightNASA @skliper @dmknutsen