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

Potential Truncation of Number of Open Files #27

Open
dmknutsen opened this issue May 25, 2022 · 2 comments
Open

Potential Truncation of Number of Open Files #27

dmknutsen opened this issue May 25, 2022 · 2 comments

Comments

@dmknutsen
Copy link
Contributor

The number of open files is treated as a uint32 by FSW, but is telemetered as a uint8 such that truncation may result if the number of open files allowed by OSAL grows.

@dmknutsen dmknutsen added the bug label May 25, 2022
@skliper skliper added enhancement and removed bug labels May 26, 2022
@skliper
Copy link
Contributor

skliper commented May 26, 2022

Switched to enhancement since this is informational and only in the HK packet (not the get open files command response telemetry) and basically just means we only support reporting up to uint8 files in HK. Note the OSAL_CONFIG_MAX_NUM_OPEN_FILES parameter which is used to set OS_MAX_NUM_OPEN_FILES limits within OSAL the max number of open files which is currently defaulted to 50.

First off the tlm packet does support uint32:

FM/fsw/src/fm_msg.h

Lines 398 to 404 in 3a8d00a

typedef struct
{
CFE_MSG_TelemetryHeader_t TlmHeader; /**< \brief Telemetry Header */
uint32 NumOpenFiles; /**< \brief Number of files opened via cFE */
FM_OpenFilesEntry_t OpenFilesList[OS_MAX_NUM_OPEN_FILES]; /**< \brief List of files opened via cFE */
} FM_OpenFilesPkt_t;

OSAL has a config param defaulted to 50 for the maximum number of open files:
https://github.com/nasa/osal/blob/acb88caaf6507ff4bcae6efe56af81adba88252d/default_config.cmake#L258-L261

Logic for the command all supports uint32:

FM/fsw/src/fm_cmds.c

Lines 582 to 611 in 3a8d00a

bool FM_GetOpenFilesCmd(const CFE_SB_Buffer_t *BufPtr)
{
const char *CmdText = "Get Open Files";
bool CommandResult = false;
uint32 NumOpenFiles = 0;
/* Verify command packet length */
CommandResult =
FM_IsValidCmdPktLength(&BufPtr->Msg, sizeof(FM_GetOpenFilesCmd_t), FM_GET_OPEN_FILES_PKT_ERR_EID, CmdText);
if (CommandResult == true)
{
/* Initialize open files telemetry packet */
CFE_MSG_Init(&FM_GlobalData.OpenFilesPkt.TlmHeader.Msg, CFE_SB_ValueToMsgId(FM_OPEN_FILES_TLM_MID),
sizeof(FM_OpenFilesPkt_t));
/* Get list of open files and count */
NumOpenFiles = FM_GetOpenFilesData(FM_GlobalData.OpenFilesPkt.OpenFilesList);
FM_GlobalData.OpenFilesPkt.NumOpenFiles = NumOpenFiles;
/* Timestamp and send open files telemetry packet */
CFE_SB_TimeStampMsg(&FM_GlobalData.OpenFilesPkt.TlmHeader.Msg);
CFE_SB_TransmitMsg(&FM_GlobalData.OpenFilesPkt.TlmHeader.Msg, true);
/* Send command completion event (debug) */
CFE_EVS_SendEvent(FM_GET_OPEN_FILES_CMD_EID, CFE_EVS_EventType_DEBUG, "%s command", CmdText);
}
return (CommandResult);
} /* End of FM_GetOpenFilesCmd() */

Really the issue is in the HK packet (not the command response telemetry):

uint8 NumOpenFiles; /**< \brief Number of open files in the system */

FM_GlobalData.HousekeepingPkt.NumOpenFiles = FM_GetOpenFilesData(NULL);

@dmknutsen
Copy link
Contributor Author

Agreed - that is what I was trying to communicate. Thanks for the clarification!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants