nasa / FM

The Core Flight System (cFS) File Manager (FM) application.
Apache License 2.0
42 stars 24 forks source link

fm_child.c has an unreachable branch #36

Closed chillfig closed 2 years ago

chillfig commented 2 years ago

Checklist (Please check before submitting)

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)

https://github.com/nasa/FM/blob/51707f20350e4ea909842acb3597fff8da3ab18e/fsw/src/fm_child.c#L1275-L1291

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

skliper commented 2 years ago

The strange part here is FM_DirListEntry_t EntryName is of size OS_MAX_PATH_LEN, yet the EntryLength is limited by os_dirent_t Filename size of OS_MAX_FILE_NAME as long as it's NULL terminated (which it is). https://github.com/nasa/FM/blob/51707f20350e4ea909842acb3597fff8da3ab18e/fsw/src/fm_msg.h#L317-L323 https://github.com/nasa/osal/blob/2864b25fafdd314e09d5406d8f2a03369550a4aa/src/os/inc/osapi-dir.h#L32-L35

Note LocalName is also OS_MAX_PATH_LEN: https://github.com/nasa/FM/blob/51707f20350e4ea909842acb3597fff8da3ab18e/fsw/src/fm_child.c#L1199

I vote for removing the check.