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

Fix 2358 evs fmt mk2 #2382

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

CDKnightNASA
Copy link
Contributor

@CDKnightNASA CDKnightNASA commented Jun 20, 2023

Joe, for your consideration, another pass at the "%f" and strftime code. This simplifies it a bit (but only supports one "%f" in the format string.)

@CDKnightNASA CDKnightNASA requested a review from jphickey June 20, 2023 22:03
@CDKnightNASA CDKnightNASA self-assigned this Jun 20, 2023
@@ -98,6 +98,8 @@
static const UT_TaskPipeDispatchId_t UT_TPID_CFE_TIME_INVALID_MID = {.MsgId = CFE_SB_MSGID_RESERVED, .CommandCode = 0};
static const UT_TaskPipeDispatchId_t UT_TPID_CFE_TIME_CMD_INVALID_CC = {
.MsgId = CFE_SB_MSGID_WRAP_VALUE(CFE_TIME_CMD_MID), .CommandCode = 0x7F};
static const UT_TaskPipeDispatchId_t UT_TPID_CFE_TIME_SET_PRINT_CC = {

Check notice

Code scanning / CodeQL

Variable scope too large

The variable UT_TPID_CFE_TIME_SET_PRINT_CC is only accessed in [Test_PipeCmds](1) and should be scoped accordingly.
@@ -240,6 +240,13 @@
}
break;

case CFE_TIME_SET_PRINT_CC:
if (CFE_TIME_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CFE_TIME_SetPrintCmd_t)))

Check warning

Code scanning / CodeQL

Side effect in a Boolean expression

This Boolean expression is not side-effect free.
* See description in header file for argument/return detail
*
*-----------------------------------------------------------------*/
int32 CFE_TIME_SetPrintFormat(CFE_TIME_PrintTimestamp_Enum_t PrintTimestamp, const char *PrintFormat)

Check notice

Code scanning / CodeQL

Long function without assertion

All functions of more than 10 lines should have at least one assertion.
Comment on lines +1078 to +1093
while (PctF[0] != '\0') {
if (PctF[0] == '%')
{
switch (PctF[1])
{
case 'f':
CFE_TIME_Global.PrintFormatMillis = PctF;
break;
case '\0':
break;
default:
PctF++; /* skip forward two chars */
}
}
PctF++;
}

Check warning

Code scanning / CodeQL

Unbounded loop

This loop does not have a fixed bound.
Comment on lines +581 to +640
switch (CFE_TIME_Global.PrintTimestamp)
{
case CFE_TIME_PrintTimestamp_DateTime:
gmtime_r(&sec, &tm);

/*
** `PrintFormatMillis` points at the `%f` in PrintFormat, if there is a `%f`.
*/
if (CFE_TIME_Global.PrintFormatMillis)
{
/* ...blot out the `%f`, temporarily. */
CFE_TIME_Global.PrintFormatMillis[0] = '\0';
}

TimeSz = strftime(PrintBuffer, CFE_TIME_PRINTED_STRING_SIZE, CFE_TIME_Global.PrintFormat, &tm);

if (TimeSz == 0)
{
return CFE_TIME_FORMAT_TOO_LONG;
}

OutChrs += TimeSz;

if (CFE_TIME_Global.PrintFormatMillis)
{
/* unblot */
CFE_TIME_Global.PrintFormatMillis[0] = '%';

if (OutChrs + 6 > CFE_TIME_PRINTED_STRING_SIZE)
{
return CFE_TIME_FORMAT_TOO_LONG;
}

OutChrs += snprintf(PrintBuffer + OutChrs, CFE_TIME_PRINTED_STRING_SIZE - OutChrs, "%05d", mic);

/* it's likely the `%f` is last in the format string, check if there's any remainder...*/
if (CFE_TIME_Global.PrintFormatMillis[2])
{
TimeSz = strftime(PrintBuffer + OutChrs, CFE_TIME_PRINTED_STRING_SIZE, CFE_TIME_Global.PrintFormatMillis + 2, &tm);

if (TimeSz == 0)
{
return CFE_TIME_FORMAT_TOO_LONG;
}

OutChrs += TimeSz;
}
}
PrintBuffer[OutChrs] = '\0';
break;

case CFE_TIME_PrintTimestamp_SecsSinceStart:
OutChrs += snprintf(PrintBuffer, CFE_TIME_PRINTED_STRING_SIZE, "%ld.%06d", (long int)sec, mic);
PrintBuffer[OutChrs] = '\0';
break;

default:
PrintBuffer[0] = '\0';
break;
}

Check notice

Code scanning / CodeQL

Long switch case

Switch has at least one case that is too long: [CFE_TIME_PrintTimestamp_DateTime (48 lines)](1).
*-----------------------------------------------------------------*/
int32 CFE_TIME_SetPrintFormat(CFE_TIME_PrintTimestamp_Enum_t PrintTimestamp, const char *PrintFormat)
{
if (PrintTimestamp < 0 || PrintTimestamp > CFE_TIME_PrintTimestamp_None)

Check warning

Code scanning / CodeQL

Comparison result is always the same

Comparison is always false because PrintTimestamp >= 0.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant