From 137768d16f5e06a3d3119d3e5249cb812204ac81 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Fri, 15 Jan 2021 16:20:31 -0500 Subject: [PATCH] Fix #982, use separate pipe info data struct Define a data structure in cfe_sb_msg.h that will be used with the "write pipe info" command (CFE_SB_SEND_PIPE_INFO_CC). This allows the internal CFE_SB_PipeD_t descriptor object to evolve as needed without affecting the binary format of the file that is generated form this command, and items such as memory pointers may be excluded from the file. --- fsw/cfe-core/src/inc/cfe_sb_msg.h | 30 ++++++++++++++++++++++++++ fsw/cfe-core/src/sb/cfe_sb_task.c | 35 ++++++++++++++++++++++++++----- 2 files changed, 60 insertions(+), 5 deletions(-) diff --git a/fsw/cfe-core/src/inc/cfe_sb_msg.h b/fsw/cfe-core/src/inc/cfe_sb_msg.h index 40623ad8d..953f8b4a5 100644 --- a/fsw/cfe-core/src/inc/cfe_sb_msg.h +++ b/fsw/cfe-core/src/inc/cfe_sb_msg.h @@ -615,6 +615,36 @@ typedef struct CFE_SB_PipeDepthStats { }CFE_SB_PipeDepthStats_t; +/** +** \brief SB Pipe Information File Entry +** +** This statistics structure is output as part of the CFE SB +** "Send Pipe Info" command (CFE_SB_SEND_PIPE_INFO_CC). +** +** Previous versions of CFE simply wrote the internal CFE_SB_PipeD_t object +** to the file, but this also contains information such as pointers which are +** not relevant outside the running CFE process. +** +** By defining the pipe info structure separately, it also provides some +** independence, such that the internal CFE_SB_PipeD_t definition +** can evolve without changing the binary format of the information +** file. +*/ +typedef struct CFE_SB_PipeInfoEntry +{ + CFE_SB_PipeId_t PipeId; /**< The runtime ID of the pipe */ + CFE_ES_ResourceID_t AppId; /**< The runtime ID of the application that owns the pipe */ + char PipeName[CFE_MISSION_MAX_API_LEN]; /**< The Name of the pipe */ + char AppName[CFE_MISSION_MAX_API_LEN]; /**< The Name of the application that owns the pipe */ + uint16 MaxQueueDepth; /**< The allocated depth of the pipe (max capacity) */ + uint16 CurrentQueueDepth; /**< The current depth of the pipe */ + uint16 PeakQueueDepth; /**< The peak depth of the pipe (high watermark) */ + uint16 SendErrors; /**< Number of errors when writing to this pipe */ + uint8 Opts; /**< Pipe options set (bitmask) */ + uint8 Spare[3]; /**< Padding to make this structure a multiple of 4 bytes */ + +} CFE_SB_PipeInfoEntry_t; + /** ** \cfesbtlm SB Statistics Telemetry Packet ** diff --git a/fsw/cfe-core/src/sb/cfe_sb_task.c b/fsw/cfe-core/src/sb/cfe_sb_task.c index dc2cd0db9..1c09a6fe8 100644 --- a/fsw/cfe-core/src/sb/cfe_sb_task.c +++ b/fsw/cfe-core/src/sb/cfe_sb_task.c @@ -1141,7 +1141,8 @@ int32 CFE_SB_SendPipeInfo(const char *Filename) uint32 EntryCount = 0; CFE_FS_Header_t FileHdr; CFE_SB_PipeD_t *PipeDscPtr; - CFE_SB_PipeD_t entry; /* NOTE: Should be separate/dedicated type */ + CFE_SB_PipeInfoEntry_t FileEntry; + osal_id_t SysQueueId; Status = OS_OpenCreate(&fd, Filename, OS_FILE_FLAG_CREATE | OS_FILE_FLAG_TRUNCATE, OS_WRITE_ONLY); @@ -1172,14 +1173,38 @@ int32 CFE_SB_SendPipeInfo(const char *Filename) { if (CFE_SB_PipeDescIsUsed(PipeDscPtr)) { - memcpy(&entry, PipeDscPtr, sizeof(CFE_SB_PipeD_t)); + /* + * Ensure any old data in the struct has been cleared + */ + memset(&FileEntry, 0, sizeof(FileEntry)); + + /* + * Take a "snapshot" of the PipeDsc state while locked + */ + FileEntry.PipeId = CFE_SB_PipeDescGetID(PipeDscPtr); + FileEntry.AppId = PipeDscPtr->AppId; + FileEntry.MaxQueueDepth = PipeDscPtr->QueueDepth; + FileEntry.CurrentQueueDepth = PipeDscPtr->CurrentDepth; + FileEntry.PeakQueueDepth = PipeDscPtr->PeakDepth; + FileEntry.SendErrors = PipeDscPtr->SendErrors; + FileEntry.Opts = PipeDscPtr->Opts; + SysQueueId = PipeDscPtr->SysQueueId; CFE_SB_UnlockSharedData(__FILE__,__LINE__); - Status = OS_write (fd, &entry, sizeof(CFE_SB_PipeD_t)); - if (Status != sizeof(CFE_SB_PipeD_t)) + /* + * Gather data from other subsystems while unlocked. + * This might fail if the pipe is deleted simultaneously while this runs, but in + * the unlikely event that happens, the name data will simply be blank as the ID(s) + * will not validate. + */ + OS_GetResourceName(SysQueueId, FileEntry.PipeName, sizeof(FileEntry.PipeName)); + CFE_ES_GetAppName(FileEntry.AppName, FileEntry.AppId, sizeof(FileEntry.AppName)); + + Status = OS_write (fd, &FileEntry, sizeof(FileEntry)); + if (Status != sizeof(FileEntry)) { - CFE_SB_FileWriteByteCntErr(Filename,sizeof(CFE_SB_PipeD_t),Status); + CFE_SB_FileWriteByteCntErr(Filename,sizeof(FileEntry),Status); OS_close(fd); return CFE_SB_FILE_IO_ERR; }/* end if */