Merge pull request nasa#865 from CDKnightNASA/fix-862-unsub_of_unsubd

fix nasa#862 - unsub of a message ID that is already unsubbed
jphickey · Sep 10, 2020 · fd85c70 · fd85c70
2 parents bc48dc0 + be1a23a
commit fd85c70
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 27 deletions.
diff --git a/fsw/cfe-core/src/sb/cfe_sb_api.c b/fsw/cfe-core/src/sb/cfe_sb_api.c
@@ -1019,7 +1019,6 @@ int32 CFE_SB_UnsubscribeFull(CFE_SB_MsgId_t MsgId,CFE_SB_PipeId_t PipeId,
     CFE_SB_RouteEntry_t* RoutePtr;
     uint32  PipeIdx;
     uint32  TskId = 0;
-    bool    MatchFound = false;
     CFE_SB_DestinationD_t   *DestPtr = NULL;
     char    FullName[(OS_MAX_API_NAME * 2)];
 
@@ -1067,8 +1066,9 @@ int32 CFE_SB_UnsubscribeFull(CFE_SB_MsgId_t MsgId,CFE_SB_PipeId_t PipeId,
     MsgKey = CFE_SB_ConvertMsgIdtoMsgKey(MsgId);
     RouteIdx = CFE_SB_GetRoutingTblIdx(MsgKey);
 
-    /* if there are no subscriptions for this message id... */
-    if(!CFE_SB_IsValidRouteIdx(RouteIdx)){
+    /* if there have never been subscriptions for this message id... */
+    if(!CFE_SB_IsValidRouteIdx(RouteIdx))
+    {
         char    PipeName[OS_MAX_API_NAME] = {'\0'};
 
         CFE_SB_UnlockSharedData(__func__,__LINE__);
@@ -1082,42 +1082,43 @@ int32 CFE_SB_UnsubscribeFull(CFE_SB_MsgId_t MsgId,CFE_SB_PipeId_t PipeId,
         return CFE_SUCCESS;
     }/* end if */
 
-    /* At this point, there must be at least one destination. */
-    /* So the value of 'ListHeadPtr' will not be NULL by design */
     RoutePtr = CFE_SB_GetRoutePtrFromIdx(RouteIdx);
 
     /* search the list for a matching pipe id */
-    DestPtr = RoutePtr->ListHeadPtr;
-
-    do{
+    for (DestPtr = RoutePtr->ListHeadPtr; DestPtr != NULL && DestPtr->PipeId != PipeId; DestPtr = DestPtr->Next)
+        ;
 
-        if(DestPtr->PipeId == PipeId){
-            /* match found, remove node from list */
-            CFE_SB_RemoveDest(RoutePtr,DestPtr);
-
-            /* return node to memory pool */
-            CFE_SB_PutDestinationBlk(DestPtr);
+    if(DestPtr)
+    {
+        /* match found, remove node from list */
+        CFE_SB_RemoveDest(RoutePtr,DestPtr);
 
-            RoutePtr->Destinations--;
-            CFE_SB.StatTlmMsg.Payload.SubscriptionsInUse--;
+        /* return node to memory pool */
+        CFE_SB_PutDestinationBlk(DestPtr);
 
-            MatchFound = true;
+        RoutePtr->Destinations--;
+        CFE_SB.StatTlmMsg.Payload.SubscriptionsInUse--;
 
-        }/* end if */
+        CFE_EVS_SendEventWithAppID(CFE_SB_SUBSCRIPTION_REMOVED_EID,CFE_EVS_EventType_DEBUG,CFE_SB.AppId,
+            "Subscription Removed:Msg 0x%x on pipe %d,app %s",
+            (unsigned int)CFE_SB_MsgIdToValue(MsgId),
+            (int)PipeId,CFE_SB_GetAppTskName(TskId,FullName));
+    }
+    else
+    {
+        char    PipeName[OS_MAX_API_NAME] = {'\0'};
 
-        DestPtr = DestPtr->Next;
+        CFE_SB_GetPipeName(PipeName, sizeof(PipeName), PipeId);
 
-    }while((MatchFound == false)&&(DestPtr != NULL));
+        CFE_EVS_SendEventWithAppID(CFE_SB_UNSUB_NO_SUBS_EID,CFE_EVS_EventType_INFORMATION,CFE_SB.AppId,
+            "Unsubscribe Err:No subs for Msg 0x%x on %s,app %s",
+            (unsigned int)CFE_SB_MsgIdToValue(MsgId),
+            PipeName,CFE_SB_GetAppTskName(TskId,FullName));
+    }/* end if */
 
     CFE_SB_UnlockSharedData(__func__,__LINE__);
 
-    CFE_EVS_SendEventWithAppID(CFE_SB_SUBSCRIPTION_REMOVED_EID,CFE_EVS_EventType_DEBUG,CFE_SB.AppId,
-            "Subscription Removed:Msg 0x%x on pipe %d,app %s",
-            (unsigned int)CFE_SB_MsgIdToValue(MsgId),
-            (int)PipeId,CFE_SB_GetAppTskName(TskId,FullName));
-
     return CFE_SUCCESS;
-
 }/* end CFE_SB_UnsubscribeFull */
 
 /*

diff --git a/fsw/cfe-core/unit-test/sb_UT.c b/fsw/cfe-core/unit-test/sb_UT.c
@@ -2545,7 +2545,7 @@ void Test_Unsubscribe_NoMatch(void)
     CFE_SB.RoutingTbl[CFE_SB_RouteIdxToValue(Idx)].ListHeadPtr->Next = NULL;
     ASSERT(CFE_SB_Unsubscribe(MsgId, TestPipe));
 
-    EVTCNT(6);
+    EVTCNT(7);
 
     EVTSENT(CFE_SB_UNSUB_NO_SUBS_EID);