Giter VIP home page Giter VIP logo

fm's Introduction

core Flight System (cFS) File Manager Application (FM)

Introduction

The File Manager application (FM) is a core Flight System (cFS) application that is a plug-in to the Core Flight Executive (cFE) component of the cFS.

The FM application provides onboard file system management services by processing ground commands for copying, moving, and renaming files, decompressing files, creating directories, deleting files and directories, providing file and directory informational telemetry messages, and providing open file and directory listings.

The FM application is written in C and depends on the cFS Operating System Abstraction Layer (OSAL) and cFE components. There is additional FM application specific configuration information contained in the application user's guide.

User's guide information can be generated using Doxygen (from top mission directory):

  make prep
  make -C build/docs/fm-usersguide fm-usersguide

Software Required

cFS Framework (cFE, OSAL, PSP)

An integrated bundle including the cFE, OSAL, and PSP can be obtained at https://github.com/nasa/cfs

About cFS

The cFS is a platform and project independent reusable software framework and set of reusable applications developed by NASA Goddard Space Flight Center. This framework is used as the basis for the flight software for satellite data systems and instruments, but can be used on other embedded systems. More information on the cFS can be found at http://cfs.gsfc.nasa.gov

fm's People

Contributors

astrogeco avatar chillfig avatar dmknutsen avatar dzbaker avatar ejtimmon avatar havencarlson avatar jasonduley avatar jphickey avatar skliper avatar the-other-james avatar thnkslprpt avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

fm's Issues

Internal API calls are not const-correct (possible race condition)

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I performed a cursory search to see if the bug report is relevant, not redundant, nor in conflict with other tickets.

Describe the bug
Functions that accept a pointer as an input only, and do not modify that structure, should be qualified as const.

In particular, this is true of the software bus messages - these could be distributed to multiple subscribers, and these must not be modified by receivers or else race conditions are introduced.

There is at least one case of FM where the buffer is modified. This should be corrected along with adding const to all messages.

To Reproduce
Possible race condition if multiple entities subscribe to the same message (config-dependent)

Expected behavior
Should not modify input. Non-modified input structs should be const in the declarations.

Code snips
Possible case of input buffer getting modified is here:

FM/fsw/src/fm_cmd_utils.c

Lines 350 to 354 in c856997

default: /* FilenameState == FM_NAME_IS_INVALID */
/* Insert a terminator in case the invalid string did not have one */
Filename[BufferSize - 1] = '\0';
ErrorCode = FM_FNAME_INVALID_EID_OFFSET;
strncpy(Spec, "%s error: filename is invalid: name = %s", SpecLen - 1);

System observed on:
N/A

Additional context
Pretty much most/all of the functions in fm_cmd_utils.h should accept const string inputs.

Reporter Info
Joseph Hickey, Vantage Systems, Inc.

FM should use CFE_FS_InitHeader

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
FM initializes and populates the CFE FS header itself
This requires FM to understand the details of the header structure and could break if that structure changes.

Describe the solution you'd like
FM should instead use the CFE_FS_InitHeader function which is designed to do exactly this.

Requester Info
Haven Carlson

Style warnings in strict cppcheck analysis

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
Currently fails cppcheck strict checking in static analysis workflow:
https://github.com/nasa/FM/actions/runs/2282818788

[fsw/src/fm_cmd_utils.h:214] -> [fsw/src/fm_cmd_utils.c:372]: (style, inconclusive) Function 'FM_VerifyFileNoExist' argument 1 names different: declaration 'Name' definition 'Filename'.
[fsw/src/fm_cmd_utils.h:236] -> [fsw/src/fm_cmd_utils.c:418]: (style, inconclusive) Function 'FM_VerifyFileNotOpen' argument 1 names different: declaration 'Name' definition 'Filename'.
[fsw/src/fm_tbl.c:43] -> [fsw/src/fm_tbl.c:49]: (style) Variable 'Status' is reassigned a value before the old one has been used.
[fsw/src/fm_tbl.c:191] -> [fsw/src/fm_tbl.c:197]: (style) Variable 'Status' is reassigned a value before the old one has been used.
[fsw/src/fm_tbl.h:73] -> [fsw/src/fm_tbl.c:72]: (style, inconclusive) Function 'FM_ValidateTable' argument 1 names different: declaration 'TableData' definition 'TablePtr'.
Error: Process completed with exit code 255.

Describe the solution you'd like
Fix

Describe alternatives you've considered
None

Additional context
None

Requester Info
Jacob Hageman - NASA/GSFC

Clarify command counter requirements allow for multiple contexts (tasks)

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
Some of the command acceptance check requirements are implemented at in the child task. For those requirements, it's the child task command error counter that increments instead of the main task command error counter. This isn't really clear in FM1004 which implies a single error counter.

Describe the solution you'd like
Since the concept is the same and it's somewhat implementation dependent (there's no requirement for a child task) the suggestion is to update the rationale to state this can be the case.

Describe alternatives you've considered
Could update each requirement that is handled by the child task, but also could result in over-specifying the design.

Additional context
None

Requester Info
Jacob Hageman - NASA/GSFC

FM Dir List to file doesn't clean buffer

FM Get Directory List to file (FM_GET_DIR_FILE_CC) currently isn't zeroing out the buffer of DirListData.EntryName so there can be a bunch of random stuff in there.

The DirListData that is written currently looks like this:
EntryName: fm_dirlist.out\00\00/dev/shmw��\00\00hw��\00\00 j��\00\00��\00\007���
EntrySize: 136
ModifyTime: 1638892240
Mode: 6

When it it should look like this:
EntryName: fm_dirlist.out
EntrySize: 136
ModifyTime: 1638892240
Mode: 6

The code looks like this, the commented out part is what is missing and would fix the problem.

/* Populate directory list file entry */
//CFE_PSP_MemSet(&DirListData, 0, sizeof(FM_DirListEntry_t));
strncpy(DirListData.EntryName, OS_DIRENTRY_NAME(DirEntry), EntryLength);
DirListData.EntryName[EntryLength] = '\0';

Imported from GSFCCFS-1815

Update CLA Information

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the cFS README.md file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
Have new CLAs given the change in nasa/cFS#491 with the combined CLA,

Describe the solution you'd like

  • Update the instructions in each app's Contributing.md
  • Delete old CLA pdfs
  • Update PR and Issue templates as needed

Describe alternatives you've considered

None

Additional context
None

Requester Info
Gerardo E. Cruz-Ortiz

fm_child.c has an unreachable branch

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I performed a cursory search to see if the bug report is relevant, not redundant, nor in conflict with other tickets.

Describe the bug
OS_MAX_FILE_NAME is immutable in testing code coverage. Therefore, full branch coverage starting on line 1275 below is not possible.

Example Code Coverage result:

    1274                 :            :                     /* Verify combined directory plus filename length */
    1275 [ +  - ][ -  + ]:          1 :                     if ((EntryLength < sizeof(ListEntry->EntryName)) && ((PathLength + EntryLength) < OS_MAX_PATH_LEN))
    1276                 :            :                     {
    1277                 :            :                         /* Add filename to directory listing telemetry packet */
    1278                 :          0 :                         strncpy(ListEntry->EntryName, OS_DIRENTRY_NAME(DirEntry), EntryLength);
    1279                 :          0 :                         ListEntry->EntryName[EntryLength] = '\0';
    1280                 :            : 
    1281                 :            :                         /* Build filename - Directory already has path separator */
    1282                 :          0 :                         strncpy(LogicalName, CmdArgs->Source2, PathLength);
    1283                 :          0 :                         LogicalName[PathLength] = '\0';
    1284                 :            : 
    1285                 :          0 :                         strncat(LogicalName, OS_DIRENTRY_NAME(DirEntry), (OS_MAX_PATH_LEN - PathLength));
    1286                 :            : 
    1287                 :          0 :                         FM_ChildSleepStat(LogicalName, ListEntry, &FilesTillSleep, CmdArgs->GetSizeTimeMode);
    1288                 :            : 
    1289                 :            :                         /* Add another entry to the telemetry packet */
    1290                 :          0 :                         FM_GlobalData.DirListPkt.PacketFiles++;
    1291                 :            :                     }

To Reproduce
The following expression can't be achieved:
EntryLength > sizeof(ListEntry->EntryName)

FM/fsw/src/fm_child.c

Lines 1275 to 1291 in 51707f2

if ((EntryLength < sizeof(ListEntry->EntryName)) && ((PathLength + EntryLength) < OS_MAX_PATH_LEN))
{
/* Add filename to directory listing telemetry packet */
strncpy(ListEntry->EntryName, OS_DIRENTRY_NAME(DirEntry), EntryLength);
ListEntry->EntryName[EntryLength] = '\0';
/* Build filename - Directory already has path separator */
strncpy(LogicalName, CmdArgs->Source2, PathLength);
LogicalName[PathLength] = '\0';
strncat(LogicalName, OS_DIRENTRY_NAME(DirEntry), (OS_MAX_PATH_LEN - PathLength));
FM_ChildSleepStat(LogicalName, ListEntry, &FilesTillSleep, CmdArgs->GetSizeTimeMode);
/* Add another entry to the telemetry packet */
FM_GlobalData.DirListPkt.PacketFiles++;
}

Expected behavior
100% coverage.

Code snips
See code snip above

System observed on:
Continuous Integration

Additional context
Add any other context about the problem here.

Reporter Info
Justin Figueroa, ASRC Federal

Move "FM_GlobalData" back into private/local data structures

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
The FM_GlobalData object is only used within the FM app for private data storage. It should not be visible externally. However, it is currently defined here in the public fm_msg.h file:

FM/fsw/inc/fm_msg.h

Lines 542 to 594 in c856997

typedef struct
{
FM_MonitorTable_t *MonitorTablePtr; /**< \brief File System Table Pointer */
CFE_TBL_Handle_t MonitorTableHandle; /**< \brief File System Table Handle */
CFE_SB_PipeId_t CmdPipe; /**< \brief cFE software bus command pipe */
CFE_ES_TaskId_t ChildTaskID; /**< \brief Child task ID */
osal_id_t ChildSemaphore; /**< \brief Child task wakeup counting semaphore */
osal_id_t ChildQueueCountSem; /**< \brief Child queue counter mutex semaphore */
uint8 ChildCmdCounter; /**< \brief Child task command success counter */
uint8 ChildCmdErrCounter; /**< \brief Child task command error counter */
uint8 ChildCmdWarnCounter; /**< \brief Child task command warning counter */
uint8 ChildWriteIndex; /**< \brief Array index for next write to command args */
uint8 ChildReadIndex; /**< \brief Array index for next read from command args */
uint8 ChildQueueCount; /**< \brief Number of pending commands in queue */
uint8 CommandCounter; /**< \brief Application command success counter */
uint8 CommandErrCounter; /**< \brief Application command error counter */
uint8 Spare8a; /**< \brief Placeholder for unused command warning counter */
uint8 ChildCurrentCC; /**< \brief Command code currently executing */
uint8 ChildPreviousCC; /**< \brief Command code previously executed */
uint8 Spare8b; /**< \brief Structure alignment spare */
uint32 FileStatTime; /**< \brief Modify time from most recent OS_stat */
uint32 FileStatSize; /**< \brief File size from most recent OS_stat */
uint32 FileStatMode; /**< \brief File mode from most recent OS_stat (OS_FILESTAT_MODE) */
FM_DirListFileStats_t DirListFileStats; /**< \brief Get dir list to file statistics structure */
FM_DirListPkt_t DirListPkt; /**< \brief Get dir list to packet telemetry packet */
FM_MonitorReportPkt_t
MonitorReportPkt; /**< \brief Telemetry packet reporting status of items in the monitor table */
FM_FileInfoPkt_t FileInfoPkt; /**< \brief Get file info telemetry packet */
FM_OpenFilesPkt_t OpenFilesPkt; /**< \brief Get open files telemetry packet */
FM_HousekeepingPkt_t HousekeepingPkt; /**< \brief Application housekeeping telemetry packet */
char ChildBuffer[FM_CHILD_FILE_BLOCK_SIZE]; /**< \brief Child task file I/O buffer */
FM_ChildQueueEntry_t ChildQueue[FM_CHILD_QUEUE_DEPTH]; /**< \brief Child task command queue */
#ifdef FM_INCLUDE_DECOMPRESS
FS_LIB_Decompress_State_t DecompressState;
#endif
} FM_GlobalData_t;

Describe the solution you'd like
This should be defined in one of the internal header files, not in a public interface file.

Additional context
Public API should generally only be constants / #define's, and typedefs. API calls only for libraries - apps do not have public API calls. Extern data structs / globals should not be exposed in either apps or libs for a variety of reasons.

Requester Info
Joseph Hickey, Vantage Systems, Inc.

Move interface definition files to "inc" location

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
The interface definition files of all open source apps currently exist in the "src" directory.

Describe the solution you'd like
Create an "inc" dir to go with the "src" dir. Move the interface definitions into this location: "_msg.h", "_msgdefs.h", "_tbldefs.h", and "_events.h". Consider moving header files in both "platform_inc" and "mission_inc" to "inc"

Describe alternatives you've considered
Leaving as is.

Additional context
N/A

Requester Info
Justin Figueroa, Vantage Systems

Reduce redundant code in fm_cmd_utils.c

Most verify functions in fm_cmd_utils contain several instances of checking the FilenameState. Could that code be refactored into its own function that receives the set of valid return codes, and is able to validate the return code or report the errors (maybe a bitmap). This would eliminate a big portion of redundant code. Or could it use switch statements?

Imported from GSFCCFS-1026

Update code-coverage count and add to workflow

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
N/a

Describe the solution you'd like
When #35 and #38 are merged, recompute coverage and update the limits in the code-coverage workflow

Describe alternatives you've considered
None

Additional context
None

Requester Info
Gerardo E. Cruz-Ortiz, NASA

Replace FM Internal Command Code with Internal MID instead

Currently, the FM app has an "internal" command code defined for a delete file request that originates from another app instead of the ground. However, the definition for that command code (FM_DELETE_INT_CC) is located in a the header file fm_msgdefs.h, which is located in the app's "fsw/src" directory, and strictly speaking, not accessible by other apps.

Instead of using an internal command code, FM could use an internal message ID (e.g., FM_INTERNAL_CMD_MID) that can be defined in fm_msgids.h, which is located in the platform_inc directory. Other cFS apps could then access that command MID and send the internal delete command to the FM app without reaching into what should be an FM-local header file.

Imported from GSFCCFS-965

Add CodeQL to repository

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
Use CodeQL for continuous integration

Describe the solution you'd like
Add CodeQL workflow

Describe alternatives you've considered
None

Additional context
Add any other context about the feature request here.

Requester Info
Haven Carlson

Inconsistent Event ID naming

Checklist

  • I reviewed the Contributing Guide.
  • I performed a cursory search to see if the bug report is relevant, not redundant, nor in conflict with other tickets.

Describe the bug
Copy of nasa/cFE#2175
After finding that there were 9 different Event IDs to indicate the same thing (Invalid Message ID) in nasa/CF#262, I scrubbed the other common commands (e.g. Task Initialisation [INIT], NOOP, Reset Counters etc.) and found the same issue there - almost every component/app had their own variation of the Event ID name for the exact same event.

Expected behavior
Apply consistent Event ID names to the events which are common to all/most components and apps.

Code snips
Invalid Message ID:
CFE_EVS_ERR_MSGID_EID
CFE_SB_BAD_MSGID_EID
CFE_TIME_ID_ERR_EID
CS_MID_ERR_EID
TO_LAB_MSGID_ERR_EID
SAMPLE_APP_INVALID_MSGID_ERR_EID
BP_INVALID_MID_ERR_EID
SCH_MD_ERR_EID
CI_LAB_COMMAND_ERR_EID

Initialization:
CFE_TIME_INIT_EID
CFE_TBL_INIT_INF_EID
CFE_EVS_STARTUP_EID
CF_EID_INF_INIT
BP_INIT_APP_INFO_EID
SCH_INITSTATS_INF_EID
CI_LAB_STARTUP_INF_EID

NOOP:
CFE_TIME_NOOP_EID
CFE_TBL_NOOP_INF_EID
CFE_SB_CMD0_RCVD_EID
CF_EID_INF_CMD_NOOP
FM_NOOP_CMD_EID
CI_LAB_COMMANDNOP_INF_EID

Reset Counters:
CFE_TIME_RESET_EID
CFE_TBL_RESET_INF_EID
CFE_EVS_RSTCNT_EID
CFE_SB_CMD1_RCVD_EID
CF_EID_INF_CMD_RESET
SC_RESET_DEB_EID
HS_RESET_DBG_EID
FM_RESET_CMD_EID
HK_RESET_CNTRS_CMD_EID
MD_RESET_CNTRS_DBG_EID
CI_LAB_COMMANDRST_INF_EID

etc.

Reporter Info
Avi Weiss @thnkslprpt

Remove `static` designation from CFE_TBL_FileDef objects

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
Having a global declared as static is supposed to be for cases where the symbol is not to be referenced outside of that compilation unit (that is, it makes the symbol "hidden", so to speak). It prevents other compilation units being linked with this unit from seeing the symbol.

Somewhere along the way, this keyword got applied to table definitions in many CFS apps. This is not required and even a bad idea for several reasons:

  1. Technically, the elf2cfetbl tool does need to find the symbol. (Thus if it is truly "hidden" it will not work - this seems to rely on the name still appearing in the ELF but simply as a different type of symbol)
  2. Tables are not linked with other compilation units (and thus no reason/justification on why the symbol name would ever need to be hidden)
  3. The compiler warns about it (because it is a global that does not appear to be used anywhere, at least in the compiler's view)
  4. In order to get around the previous issue and fix the warning, an __attribute__((__used__)) was added. Attributes like this are a non-standardized extension, and these are not supposed to be used in FSW code.

Describe the solution you'd like
Just make it a regular (non-static) symbol with no special attributes - everything "just works" this way.

Additional context
See

static CFE_TBL_FileDef_t CFE_TBL_FileDef
__attribute__((__used__)) = {"FM_MonitorTable", FM_APP_NAME "." FM_TABLE_CFE_NAME, FM_TABLE_DEF_DESC,

Requester Info
Joseph Hickey, Vantage Systems, Inc.

Redundant comments (/* end of function */, /* end if */ etc.) and clean up empty lines.

Checklist

  • I reviewed the Contributing Guide.
  • I performed a cursory search to see if the bug report is relevant, not redundant, nor in conflict with other tickets.

Describe the bug
Copy of nasa/to_lab#68 and nasa/sample_app#111
There are quite a few redundant comments in the code, such as:

  • /* end of function */-type comments
  • /* end if */-type comments
  • function header comments which include the function name

Another minor issue has to do with empty lines:
a) unnecessary empty lines (e.g. first line after the opening brace of a function/struct, or the last line before the closing brace - the latter apparently sometimes triggers the CI format checks).
b) missing empty lines between functions (i.e. closing brace of last function, then next function beginning on the immediately next line without an empty line in between)

The unnecessary empty lines (at the beginning or end of a function, for example) represent a low single-digit percentage of the cases (the vast majority of functions/structs do not have these extra empty lines), so there is an argument to remove them purely for consistency, not just due to them being redundant and triggering the CI format checks.

Expected behavior
Remove redundant comments to reduce clutter and inconsistency in the code, and improve readability.

Reporter Info
@thnkslprpt

Resolve issues building users guide with Ubuntu 20.04/Doxygen 1.8.17

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the CF README.md file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
Doxygen warnings for documented empty return type

Describe the solution you'd like
Remove unnecessary documentation

Describe alternatives you've considered
None

Additional context
None

Requester Info
Jacob Hageman - NASA/GSFC

Remove random numbers from unit tests

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
Several unit tests use the UT_Utils_Any_* function to introduce randomness to tests that do not need it (and might actually be hurt by it).

See for example:

FM/unit-test/fm_app_tests.c

Lines 457 to 464 in 51707f2

FM_GlobalData.CommandCounter = UT_Utils_Any_uint8();
FM_GlobalData.CommandErrCounter = UT_Utils_Any_uint8();
FM_GlobalData.ChildCmdCounter = UT_Utils_Any_uint8();
FM_GlobalData.ChildCmdErrCounter = UT_Utils_Any_uint8();
FM_GlobalData.ChildCmdWarnCounter = UT_Utils_Any_uint8();
FM_GlobalData.ChildQueueCount = UT_Utils_Any_uint8();
FM_GlobalData.ChildCurrentCC = UT_Utils_Any_uint8();
FM_GlobalData.ChildPreviousCC = UT_Utils_Any_uint8();

And following searches
https://github.com/nasa/FM/search?q=UT_Utils_Any_uint8&type=code
https://github.com/nasa/FM/search?q=UT_Utils_Any_uint32&type=code
https://github.com/nasa/FM/search?q=UT_Utils_Any_uint32_GreaterThan%280%29%3B&type=code
https://github.com/nasa/FM/search?q=UT_Utils_Any_uint8_BetweenInclusive%3B&type=code
https://github.com/nasa/FM/search?q=UT_Utils_Any_uint8_LessThan%3B&type=code

Describe the solution you'd like
Remove use of these macros where unnecessary

Describe alternatives you've considered
None

Additional context
None

Requester Info
Gerardo E. Cruz-Ortiz | NASA

Gateway Managed Storage support telemetry

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
We need to maintain current usage tracking on a directory-by-directory bases. If FM is used to modify the contents of our directories, our tracking would not be accurate which could lead to a loss of data.

For most commands to FM, we can gain the needed information by subscribing to the FM commands and querying OSAL directly. However, FM_DELETE_CC, FM_DELETE_ALL_CC, and FM_DECOMPRESS_CC commands are a problem because we would not have foreknowledge of the files size before the command is executed by FM. This would cause us to traverse an entire directory to correct the usage tracking.

We will have multiple remote drives. With potentially Terabyte drives, traversing the directories on a remote drives is too CPU and network intensive.

Describe the solution you'd like
FM_DELETE_CC:
typedef struct
{
CFE_MSG_TelemetryHeader_t TlmHeader; /< \brief Telemetry Header */
uint32 FileSize; /
< \brief Size of the file deleted*/
char Filename[OS_MAX_PATH_LEN]; /**< \brief Delete filename */

} FM_DeleteFileTel_t;

FM_DELETE_CC:
typedef struct
{
CFE_MSG_TelemetryHeader_t TlmHeader; /< \brief Telemetry Header */
uint64 Freed; /
< \brief Bytes freed in directory*/
char Directory[OS_MAX_PATH_LEN]; /**< \brief Directory name */

} FM_DeleteAllTel_t;

FM_DECOMPRESS_CC:
typedef struct
{
CFE_MSG_TelemetryHeader_t TlmHeader; /< \brief Telemetry Header */
uint32 SourceSize; /
< \brief Size of the compressed file*/
uint32 TagetSize; /< \brief Size of the uncompressed file*/
char Source[OS_MAX_PATH_LEN]; /
< \brief Source filename */
char Target[OS_MAX_PATH_LEN]; /**< \brief Target filename */
} FM_DecompressTel_t;

Describe alternatives you've considered
Similar feedback telemetry on following would be nice but not required
FM_COPY_CC
FM_MOVE_CC
FM_CONCAT_CC

Maybe there a way to have a common telemetry packet that include function code.

Additional context
Add any other context about the feature request here.

Requester Info
Nathan Lynch JSC-ER611

Directory entries are read even after packet is full (not necessary)

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
Could simplify logic and avoid looping through entire directory even after packet is full:

FM/fsw/src/fm_child.c

Lines 1247 to 1266 in 9210c2d

while (StillProcessing == true)
{
/* Read next directory entry */
Status = OS_DirectoryRead(DirId, &DirEntry);
if (Status != OS_SUCCESS)
{
/* Stop reading directory - no more entries */
StillProcessing = false;
}
else if ((strcmp(OS_DIRENTRY_NAME(DirEntry), FM_THIS_DIRECTORY) != 0) &&
(strcmp(OS_DIRENTRY_NAME(DirEntry), FM_PARENT_DIRECTORY) != 0))
{
/* Do not count the "." and ".." directory entries */
FM_GlobalData.DirListPkt.TotalFiles++;
/* Start collecting directory entries at command specified offset */
/* Stop collecting directory entries when telemetry packet is full */
if ((FM_GlobalData.DirListPkt.TotalFiles > FM_GlobalData.DirListPkt.FirstFile) &&
(FM_GlobalData.DirListPkt.PacketFiles < FM_DIR_LIST_PKT_ENTRIES))

Describe the solution you'd like

        while (Status == OS_SUCCESS && FM_GlobalData.DirListPkt.PacketFiles < FM_DIR_LIST_PKT_ENTRIES)
        {
            /* Read next directory entry */
            Status = OS_DirectoryRead(DirId, &DirEntry);

            if (Status == OS_SUCCESS && (strcmp(OS_DIRENTRY_NAME(DirEntry), FM_THIS_DIRECTORY) != 0) &&
                     (strcmp(OS_DIRENTRY_NAME(DirEntry), FM_PARENT_DIRECTORY) != 0))
            {
                /* Do not count the "." and ".." directory entries */
                FM_GlobalData.DirListPkt.TotalFiles++;

                /* Start collecting directory entries at command specified offset */
                /* Stop collecting directory entries when telemetry packet is full */
                if (FM_GlobalData.DirListPkt.TotalFiles > FM_GlobalData.DirListPkt.FirstFile)
                {

Describe alternatives you've considered
None

Additional context
None

Requester Info
Jacob Hageman - NASA/GSFC

FM configuration parameter limits need clarification

A number of FM configuration parameters have limits for which the reason is obscure at best. Limits need to be re-evaluated and comments should give clear reasoning for the limit.

Imported from GSFCCFS-1062

Add fsw/src to app target

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I performed a cursory search to see if the bug report is relevant, not redundant, nor in conflict with other tickets.

Describe the bug
Cannot build cert_testbed. target_include_directories(xx PUBLIC fsw/src) is needed to build tables, etc. Revise CMakeLists.txt

To Reproduce
Build in cert_testbed

Expected behavior
Error-free build

Code snips
If applicable, add references to the software.

System observed on:

  • Ubuntu 20.04

Additional context
N/A

Reporter Info
Justin Figueroa, Vantage Systems

Reporter Info
Full name and company/organization if applicable

Revisit coverage, update to 100% code/branch or write issues where unreachable

Failure: Coverage cs lines 99.9% functions 100.0% branches 99.1%
Failure: Coverage ds lines 99.6% functions 100.0% branches 97.7%
Failure: Coverage fm lines 93.8% functions 94.0% branches 89.1%
Failure: Coverage hs lines 100.0% functions 100.0% branches 98.1%
Failure: Coverage lc lines 99.6% functions 100.0% branches 94.3%
Failure: Coverage md lines 98.4% functions 100.0% branches 96.7%
Failure: Coverage sc lines 99.9% functions 100.0% branches 99.0%

Fix where possible, elsewise Issues should document all uncovered lines/branches and disposition (why it's ok as-is)

Imported from GSFCCFS-1935

Remove CFE_PSP_MemSet and CFE_PSP_MemCpy use on addresses in RAM

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
Should just use memset/memcpy for addresses in RAM. The PSP functions serve no use in this context.

Describe the solution you'd like
Replace with memset/memcpy.

Describe alternatives you've considered
None

Additional context
None

Requester Info
Jacob Hageman - NASA/GSFC

New GCC warnings causing build failure [-Werror=stringop-overflow=]

Checklist

  • I reviewed the Contributing Guide.
  • I performed a cursory search to see if the bug report is relevant, not redundant, nor in conflict with other tickets.

Describe the bug
I believe these are newly triggered GCC warnings (treated as errors) that are now causing the standard FM Build + Run workflow to fail.

In function ‘strncpy’,
    inlined from ‘FM_ChildDirListPktCmd’ at /home/runner/work/FM/FM/apps/fm/fsw/src/fm_child.c:1227:25:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error: ‘__builtin___strncpy_chk’ specified bound depends on the length of the source argument [-Werror=stringop-overflow=]
  106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/FM/FM/apps/fm/fsw/src/fm_child.c: In function ‘FM_ChildDirListPktCmd’:
/home/runner/work/FM/FM/apps/fm/fsw/src/fm_child.c:1221:35: note: length computed here
 1221 |                     EntryLength = strlen(OS_DIRENTRY_NAME(DirEntry));
      |                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
...
In function ‘strncat’,
    inlined from ‘FM_ChildDirListPktCmd’ at /home/runner/work/FM/FM/apps/fm/fsw/src/fm_child.c:1234:25:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:136:10: error: ‘__builtin___strncat_chk’ specified bound depends on the length of the source argument [-Werror=stringop-overflow=]
  136 |   return __builtin___strncat_chk (__dest, __src, __len, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/FM/FM/apps/fm/fsw/src/fm_child.c: In function ‘FM_ChildDirListPktCmd’:
/home/runner/work/FM/FM/apps/fm/fsw/src/fm_child.c:1221:35: note: length computed here
 1221 |                     EntryLength = strlen(OS_DIRENTRY_NAME(DirEntry));
      |                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
...
In function ‘strncpy’,
    inlined from ‘FM_ChildDirListFileLoop’ at /home/runner/work/FM/FM/apps/fm/fsw/src/fm_child.c:1450:21:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error: ‘__builtin___strncpy_chk’ specified bound depends on the length of the source argument [-Werror=stringop-overflow=]
  106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/FM/FM/apps/fm/fsw/src/fm_child.c: In function ‘FM_ChildDirListFileLoop’:
/home/runner/work/FM/FM/apps/fm/fsw/src/fm_child.c:1434:31: note: length computed here
 1434 |                 EntryLength = strlen(OS_DIRENTRY_NAME(DirEntry));
      |                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

To Reproduce
Run the Build + Run GitHub Action on the current main branch FM source code.

Expected behavior
Build + Run workflow should run without errors.

Reporter Info
Avi @thnkslprpt

cFE core services can create files FM can't delete

Describe the bug
A few commands to the cFE core services, and possibly some cFS applications, are able to create files that cannot then be moved, renamed, deleted, or otherwise by FM. One example is EVS_WRITELOG2FILE (CFE_EVS_FILE_WRITE_LOG_DATA_CC). This command (and all others that take filenames in the cFE core) don't check the validity of a filename; they simply pull the filename from the command and call OS_creat() or OS_open() immediately.

FM, on the other hand, calls CFS_IsValidFilename() to check for validity of provided filenames in all commands. So, if a command like EVS_WRITELOG2FILE is used to create a file with an invalid filename, FM cannot then access that file in any way.

To Reproduce
Steps to reproduce the behavior:

  1. Use EVS_WRITELOG2FILE to create a log file in /ram with a ? in its filename.
  2. Attempt to delete this file with FM.

Expected behavior
I would expect that no cFE application, or cFE itself, should be able to create files with invalid filenames.

My suggested fix, and how we've fixed this in our local copies of cFE and OSAL, is to move the CFS_IsValidFilename() function into the OSAL, and have all OSAL file API functions perform the check before operating on files. This guarantees that nothing above the OSAL can create a file with an invalid name.

Code snips

System observed on:

  • Capella flight computer
  • OS: FreeRTOS 9.0.0
  • Versions: cFE 6.5.0, OSAL 4.2.1 (plus in-house FreeRTOS port), cfs_lib 2.2.0, FM 2.5.2

Additional context
This is another one we discovered accidentally operationally. One of our operators created an EVS log with a question mark in the name, and then we realized that we couldn't do anything with that file with FM. We ended up patching the filename check in cfs_lib with MM so it would allow for us to delete the file.

Reporter Info
Mike Stewart, Capella Space.

Add untar command to FM

Stakeholder requested that untar capability be added in cFE. In design discussions with the framework team, it was decided that it made sense to pull the decompress capability out of the framework and into a library, and to then add an untar command to the FM app.

Imported from GSFCCFS-1083

Replace quotes with angle brackets in #include statements within /inc

Describe the solution you'd like
Quotes should be replaced with angle brackets in #include statements that reside in the /inc location. This will ensure that the preprocessor selects the files pre-designated to override the default files contained within the open source cFS build release - as opposed to selecting those located in the same directory.

Requester Info
Dan Knutsen
NASA Goddard

Internal delete command support not in requirements, doesn't match description

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
FM_DELETE_INT_CC is out of family. FM is operator interface to the file system, not what other elements should use to delete files. Also strange handling in that it updates command counters but doesn't send the event message, so it would break normal/simple command counter confirmation approach for ground commands.

Describe the solution you'd like
Deprecate and/or directly remove support of this command. Other apps should use OS_remove directly instead of relying on FM. Goal is to reduce interdependency.

Describe alternatives you've considered
None

Additional context
Replaces:

Requester Info
Jacob Hageman - NASA/GSFC

cFE 6.7.1 (OSAL 5.0.1) Comparability Issues

While integrating File Manager app 2.5.2 with cFE 6.7.1 (OSAL 5.0.1) I ran into an issue. FM has a
command that allows users to receive a telemetry packet listing all of the open files. In order to do this FM needs to be able to query OSAL's file stream resource objects. The current OSAL implementation only allows a creator to query all of the resources objects by using OS_ForEachObject(). I wrote OSAL ticket #65 to recommend a more general query feature would be helpful. In OpenSatKit I added a new function OS_QueryObjectType() that allows anyone (not restricted to the creator) to query a resource type. The specific OSAL changes are below followed by the FM code that uses the function. These changes were made for OpenStaKit 2.1 that can be found at https://github.com/OpenSatKit/OpenSatKit.

This ticket can only be implemented once the OSAL is updated with a new feature that allows FM to query the resources objects.

osapi-os-core.h:

/*
** Typedef for object query OSAL callback functions. A query does not
** have to be performed by the object creator. All fields of the
** query_record are completed.
**
** This may be used by multiple APIs
*/

typedef struct
{
const char *name_entry;
uint32 creator;
uint16 refcount;
} OS_query_record_t;

typedef void (*OS_ObjQueryCallback_t)(OS_query_record_t *query_rec, void *callback_arg); //dcm - Added for OSK

/-------------------------------------------------------------------------------------/
/**

  • @brief Query an object resource type maintained by the OSAL
  • User supplied callback is called for all active resources of a particular type
  • regardless of whether the caller created the object.

*/
uint32 OS_QueryObjectType (uint32 obj_type, OS_ObjQueryCallback_t callback_ptr, OS_query_record_t *query_rec, void *callback_arg); // dcm - Added for OSK

osapi-idmap.c:

/*----------------------------------------------------------------
*

  • Function: OS_QueryObjectType
  • Purpose: Implemented per public OSAL API
  •      See description in API and header file for detail
    

-----------------------------------------------------------------/
uint32 OS_QueryObjectType (uint32 obj_type, OS_ObjQueryCallback_t callback_ptr, OS_query_record_t *query_rec, void *callback_arg)
{

uint32 obj_index;
uint32 obj_max;
uint32 obj_id;
uint32 active_obj_cnt = 0;
OS_common_record_t  *obj_rec;

obj_max = OS_GetMaxForObjectType(obj_type);
if (obj_max > 0)
{
    OS_Lock_Global_Impl(obj_type);
    obj_index = OS_GetBaseForObjectType(obj_type);
    while (obj_max > 0)
    {
        obj_rec = &OS_common_table[obj_index];
        obj_id = obj_rec->active_id;
        if (obj_id != 0) 
        {

            query_rec->name_entry = obj_rec->name_entry;
            query_rec->creator    = obj_rec->creator;
            query_rec->refcount   = obj_rec->refcount;
            
            /*
             * Handle the object - Note that we must UN-lock before callback.
             * The callback function might lock again in a different manner.
             */
             OS_Unlock_Global_Impl(obj_type);
             (*callback_ptr)(query_rec, callback_arg);
             OS_Lock_Global_Impl(obj_type);
             
             ++active_obj_cnt;

        }
        ++obj_index;
        --obj_max;
    }
    OS_Unlock_Global_Impl(obj_type);
}

return active_obj_cnt;

} /* End OS_QueryObjectType() */

fm_cmd_utils.c:

static uint32 open_file_cnt = 0;
static void LoadOpenFileData(OS_query_record_t *query_rec, void *callback_arg)
{

FM_OpenFilesEntry_t *OpenFilesData = (FM_OpenFilesEntry_t *)callback_arg;
CFE_ES_TaskInfo_t   TaskInfo;

if (OpenFilesData != (FM_OpenFilesEntry_t *) NULL)
{
    /* FDTableEntry.Path has logical filename saved when file was opened */
    strcpy(OpenFilesData[open_file_cnt].LogicalName, query_rec->name_entry);

    /* Get the name of the application that opened the file */
    CFE_PSP_MemSet(&TaskInfo, 0, sizeof(CFE_ES_TaskInfo_t));
    if (CFE_ES_GetTaskInfo(&TaskInfo, query_rec->creator) == CFE_SUCCESS)
    {
        strcpy(OpenFilesData[open_file_cnt].AppName, (char *) TaskInfo.AppName);
    } 
}
++open_file_cnt;

} /* End LoadOpenFileData() */

/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * /
/
/
/
FM utility function -- get open files data /
/
/
/
* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */

uint32 FM_GetOpenFilesData(FM_OpenFilesEntry_t *OpenFilesData)
{

OS_query_record_t query_rec;

open_file_cnt = 0;
OS_QueryObjectType (OS_OBJECT_TYPE_OS_STREAM, LoadOpenFileData, &query_rec, (void *)OpenFilesData);
   
return open_file_cnt;

} /* End FM_GetOpenFilesData */

New Cppcheck errors: '[unreadVariable]'

Checklist

  • I reviewed the Contributing Guide.
  • I performed a cursory search to see if the bug report is relevant, not redundant, nor in conflict with other tickets.

Describe the bug
Latest version of Cppcheck is issuing the following failures for FM:

fsw/src/fm_cmd_utils.c:165:32: style: Variable 'StringLength' is assigned a value that is never used. [unreadVariable]
    int32      StringLength    = 0;
                               ^
fsw/src/fm_tbl.c:74:22: style: Variable 'NameLength' is assigned a value that is never used. [unreadVariable]
    int32 NameLength = 0;
                     ^

To Reproduce
Run the current version of Cppcheck on the current main branch FM source code.

Expected behavior
Cppcheck should pass without raising any errors.

Reporter Info
Avi @thnkslprpt

Unreachable code in fm_child.c, EntryLength never larger than sizeof(DirListData.EntryName)

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
Since both DirListData.EntryName and TempName are both OS_MAX_PATH_LENGTH, and and the os_dirent_t FileName is limited by OS_MAX_FILE_NAME, the first condition below can never be false:

if ((EntryLength < sizeof(DirListData.EntryName)) && ((PathLength + EntryLength) < OS_MAX_PATH_LEN))

Describe the solution you'd like
Remove, the second condition is sufficient (add comment)

Describe alternatives you've considered
None

Additional context
None

Requester Info
Jacob Hageman - NASA/GSFC

App Requirement Issues

  1. LC3002.1.1 & LC3002.2.1 - Don’t see where event filter is able to be specified in Action Point or Watchpoint definition tables.
  2. DS8000 – DS HK packet
  3. FM4000 – FM HK packet
  4. DS3000 - Requirement Incomplete

Imported from GSFCCFS-1917

Conditional compile of Decompress File Cmd

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
There is a substantial amount of code that is conditionally compiled based on the FM_INCLUDE_DECOMPRESS platform config option. While the intent is reasonable - to not require the dependency on (large) compression routines for users that do not need this feature - the problem is that this is disabled in the default config thus in all build and test workflows - and a significant chunk of code goes untested as a result.

The FM_INCLUDE_DECOMPRESS option disables two entire FSW functions, plus 5-6 more UT functions, which is quite substantial. Code in any of these functions can become stale/invalid due to changes elsewhere in the FSW as FM is maintained. GSFC coding practices discourage large chunks of "compiled-out" code for this reason.

Describe the solution you'd like
Reduce the amount of code that is disabled by FM_INCLUDE_DECOMPRESS. When this is disabled, dispatch tables probably should still include the CC and go through the basic motions, call the handler, put it in the work queue, etc. Generally, only the actual call to FS_LIB_Decompress() needs to be disabled.

Describe alternatives you've considered
This could potentially be done as a CMake compile-time option, with a source file selection. This is the way many optional components are implemented in other modules such as OSAL (e.g. network sockets).

Additional context
Trying a basic compile of FM with FM_INCLUDE_DECOMPRESS enabled looks like its already broken. FM itself seems OK but the FS_Lib is already stale and does not compile with Draco.

Requester Info
Joseph Hickey, Vantage Systems, Inc.

Unrepeatable queue full error during FM File Info Command

The FM main task has an internal queue to pass commands to the FM child task. Most command are executed by the child task since the command execution time is unknown or variable.

A stakeholder has experience two cases where an FM command somehow broke the FM main task and FM child task communication. The FM main task says the internal queue is full and the child task says it's waiting for the next command.

In flight, this problem seemed to go away after 20 minutes and the FM child task reported the 3 queued commands had warnings. When it happened on the ground we didn't wait long enough to see if it would clear up.

It appears the sem give/take got confused. Not sure how this can happen.

Observed on system using Vxworks 6.7, CFE 6.4.2, FM 2.4.2.

Imported from GSFCCFS-941

Either FM unit testing is incorrect or an OSAL enumeration is incorrect

@mbenson1 commented on Fri Jun 19 2020

Describe the bug
I don't know if this an FM bug or an OSAL bug. I don't know what the intent on either side was. FM is failing unit tests. The first error indicates before OSAL 5.0.3-bv, FM determined if a directory entry was a directory by using the S_IFDIR macro directly, but had an #ifdef that allowed it to use an OSAL defined macro instead.

#ifdef OS_FILESTAT_ISDIR
            if (OS_FILESTAT_ISDIR(FileStatus))
#else
            if (S_ISDIR(FileStatus.FileModeBits))
#endif

With OSAL 5.0.3-bv, the OS_FILESTAT_ISDIR macro is defined and FM is using the OS_FILESTAT_ISDIR macro. The unit test sets the FileStatus.FileModeBits to 0040000 (0x4000), but the OS_FILESTAT_ISDIR tests equality against OS_FILESTAT_MODE_DIR (0x10000) so the unit tests fail. I don't know if the OSAL developer intended to use the value expected by FM (0x4000), or if the FM should be:

from:
filestats->FileModeBits = S_IFDIR;

to:

#ifdef OS_FILESTAT_ISDIR
    filestats->FileModeBits = OS_FILESTAT_MODE_DIR;
#else
    filestats->FileModeBits = S_IFDIR;
#endif

Making the change above fixes these errors, but so does changing the enumeration from:
OS_FILESTAT_MODE_DIR = 0x10000
to
OS_FILESTAT_MODE_DIR = 0x4000

To Reproduce
Steps to reproduce the behavior:

  1. Build FM unit tests with the ut_assert, hooks, and stubs from cFE 6.5.0a
  2. Run FM unit tests

Expected behavior
FM unit tests should pass.

System observed on:

  • VirtualBox
  • Ubuntu 16.04
  • FM 2.5.2, OSAL 5.0.3-bv, app ut_assert, hooks, and stubs from cFE 6.5.0a

Reporter Info
Mathew Benson
Windhover Labs, LLC
[email protected]


@skliper commented on Mon Jun 29 2020

@ejtimmon Is this resolved with the latest FM release?

FM is incompatible with recent OSAL API changes

This is regarding OSAL 5.0.3-bv. FM calls OS_opendir, OS_readdir, and OS_closedir which were removed in favor of OS_DirectoryOpen, OS_DirectoryRead, and OS_DirectoryClose. I expect to see other issues as I make fixes to pass unit testing. I will post them here.

fm_cmd_utils.c has branches that can't be reached (7/7)

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I performed a cursory search to see if the bug report is relevant, not redundant, nor in conflict with other tickets.

Describe the bug
Looking at code coverage for #17, there are several functions in fm_cmd_utils.c that have else statements that can never be reached. These functions check the return value of FM_GetFilenameState.

Total uncovered here is 7 lines 7 branches.

To Reproduce
Functions with else statements that can never be reached:

  • FM_VerifyFileClosed

    FM/fsw/src/fm_cmd_utils.c

    Lines 315 to 320 in 51707f2

    else
    {
    CFE_EVS_SendEvent((EventID + FM_FNAME_UNKNOWN_EID_OFFSET), CFE_EVS_EventType_ERROR,
    "%s error: filename has unknown state: name = %s, state = %d", CmdText, Filename,
    FilenameState);
    }

  • FM_VerifyFileExists

    FM/fsw/src/fm_cmd_utils.c

    Lines 361 to 366 in 51707f2

    else
    {
    CFE_EVS_SendEvent((EventID + FM_FNAME_UNKNOWN_EID_OFFSET), CFE_EVS_EventType_ERROR,
    "%s error: filename has unknown state: name = %s, state = %d", CmdText, Filename,
    FilenameState);
    }

  • FM_VerifyFileNoExists

    FM/fsw/src/fm_cmd_utils.c

    Lines 407 to 412 in 51707f2

    else
    {
    CFE_EVS_SendEvent((EventID + FM_FNAME_UNKNOWN_EID_OFFSET), CFE_EVS_EventType_ERROR,
    "%s error: directory name has unknown state: name = %s, state = %d", CmdText, Filename,
    FilenameState);
    }

  • FM_VerifyFileNotOpen

    FM/fsw/src/fm_cmd_utils.c

    Lines 458 to 462 in 51707f2

    {
    CFE_EVS_SendEvent((EventID + FM_FNAME_UNKNOWN_EID_OFFSET), CFE_EVS_EventType_ERROR,
    "%s error: filename has unknown state: name = %s, state = %d", CmdText, Filename,
    FilenameState);
    }

  • FM_VerifyDirExists

    FM/fsw/src/fm_cmd_utils.c

    Lines 503 to 510 in 51707f2

    else
    {
    CFE_EVS_SendEvent((EventID + FM_FNAME_UNKNOWN_EID_OFFSET), CFE_EVS_EventType_ERROR,
    "%s error: directory name has unknown state: name = %s, state = %d", CmdText, Directory,
    FilenameState);
    }
    return (Result);

  • FM_VerifyDirNoExists

    FM/fsw/src/fm_cmd_utils.c

    Lines 549 to 556 in 51707f2

    else
    {
    CFE_EVS_SendEvent((EventID + FM_FNAME_UNKNOWN_EID_OFFSET), CFE_EVS_EventType_ERROR,
    "%s error: directory name has unknown state: name = %s, state = %d", CmdText, Name,
    FilenameState);
    }
    return (Result);

Function if an if statement that can't be reached

Expected behavior
100% coverage

Code snips
See above

System observed on:
CI

Additional context
None

Reporter Info
Haven Carlson, NASA

Static analysis issues relative to flight code

Handful of static analysis issues in the "red" identified (non-Style issues). Need to resolve these.

Filter: -file:elf -file:ut -file:cfe -file:os -file:cf_ -file:_lab_app.c !(significance:style)

should resolve and/or disposition the higher ranked ones at minimum.

Note license restricts publishing issues.

Imported from GSFCCFS-1958

FM_ChildSizeTimeMode doesn't clear FileMode on OS_stat failure

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I performed a cursory search to see if the bug report is relevant, not redundant, nor in conflict with other tickets.

Describe the bug
FileMode could be uninitilized when written if there's an OS_stat failure since it's not cleared:

FM/fsw/src/fm_child.c

Lines 1587 to 1610 in 9210c2d

int32 FM_ChildSizeTimeMode(const char *Filename, uint32 *FileSize, uint32 *FileTime, uint32 *FileMode)
{
int32 Result = OS_SUCCESS;
os_fstat_t FileStatus;
memset(&FileStatus, 0, sizeof(FileStatus));
Result = OS_stat(Filename, &FileStatus);
if (Result != OS_SUCCESS)
{
*FileSize = 0;
*FileTime = 0;
}
else
{
*FileTime = OS_FILESTAT_TIME(FileStatus);
*FileSize = OS_FILESTAT_SIZE(FileStatus);
*FileMode = OS_FILESTAT_MODE(FileStatus);
}
return (Result);
} /* End of FM_ChildSizeTimeMode */

To Reproduce
Pass in uninitialized FileMode, observe not cleared on OS_stat failure.

Expected behavior
Should clear all values.

System observed on:
Observation

Additional context
Low likelihood of ever seeing this (maybe file deleted between directory read and OS_stat?), and just would write uninitialized data to the output.

Reporter Info
Jacob Hageman - NASA/GSFC

DS and FM use the same default subtype

DS and FM both use "12345" as the default file subtype

ds/fsw/platform_inc/ds_platform_cfg.h:#define DS_FILE_HDR_SUBTYPE 12345
fm/fsw/platform_inc/fm_platform_cfg.h:#define FM_DIR_LIST_FILE_SUBTYPE 12345

Imported from GSFCCFS-1735

All CMD/TLM message should put content in a "Payload" sub-structure

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
To match the patterns used in CFE and other modules, all CMD/TLM message definitions should put the content (non-header) parts into a separate struct called "Payload".

Describe the solution you'd like
Separate message content into a sub structure called "Payload".

Additional context
This is benefit to tooling that can use the presence of this field to identify where the actual content starts (e.g. something like offsetof(MsgType, Payload) would work and be correct, as opposed to checking sizeof(CFE_MSG_CommandHeader_t) which may not actually reflect where the content starts due to possible compiler-added padding between them).

Requester Info
Joseph Hickey, Vantage Systems, Inc.

Add Signature Checking Command to FM

Stakeholder request - add a command to check the signature of a file.

In the open source version of the app, this will call an empty stub in fs_lib that will always succeed (allows external users to fill in their own implementation).

Imported from GSFCCFS-1407

FM_DELETE_INT_CC appears to be redundant with FM_DELETE_CC

The doxygen comment for FM_DELETE_INT_CC states: "This is a special version of the #FM_DELETE_CC command for use when the command is sent by another application, rather than from the ground. This version of the command will not generate a success event, nor will the command increment the command success counter. The intent is to avoid confusion resulting from telemetry representing the results of delete commands sent by other applications and those sent from the ground."

However, this does not appear to be the case. Both FM_DELETE_INT_CC and FM_DELETE_CC call the same functions. It appears that FM_DELETE_INT_CC does increment the command success counter, but does not send an event message from the child task.

I think that the need for FM_DELETE_INT_CC needs to be reevaluated and if it is needed, it should be updated to match its description.

Imported from GSFCCFS-1058

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.