Server: Remove double-iteration in UA_MonitoredItem_removeNodeEventCallback

src/server/ua_subscription_events.c

@@ -13,26 +13,30 @@
 UA_StatusCode
 UA_MonitoredItem_removeNodeEventCallback(UA_Server *server, UA_Session *session,
                                          UA_Node *node, void *data) {
+    if(node->nodeClass != UA_NODECLASS_OBJECT)
+        return UA_STATUSCODE_BADINTERNALERROR;
+
+    UA_ObjectNode *on = (UA_ObjectNode*)node;
+
+    if(!on->monitoredItemQueue)
+        return UA_STATUSCODE_GOOD;
+
     /* data is the monitoredItemID */
     /* catch edge case that it's the first element */
-    if (data == ((UA_ObjectNode *) node)->monitoredItemQueue) {
-        ((UA_ObjectNode *)node)->monitoredItemQueue = ((UA_MonitoredItem *)data)->next;
+    if(data == on->monitoredItemQueue) {
+        on->monitoredItemQueue = on->monitoredItemQueue->next;
         return UA_STATUSCODE_GOOD;
     }
 
-    /* SLIST_FOREACH */
-    for (UA_MonitoredItem *entry = ((UA_ObjectNode *) node)->monitoredItemQueue->next;
-         entry != NULL; entry=entry->next) {
-        if (entry == (UA_MonitoredItem *)data) {
-            /* SLIST_REMOVE */
-            UA_MonitoredItem *iter = ((UA_ObjectNode *) node)->monitoredItemQueue;
-            for (; iter->next != entry; iter=iter->next) {}
-            iter->next = entry->next;
-            /* Unlike SLIST_REMOVE, do not free the entry, since it
-             * is still being worked on in the calling function */
+    UA_MonitoredItem *prev = on->monitoredItemQueue;
+    for(UA_MonitoredItem *entry = prev->next; entry != NULL; entry = entry->next) {
+        if(entry == (UA_MonitoredItem *)data) {
+            prev->next = entry->next;
             break;
         }
+        prev = entry;
     }
+
     return UA_STATUSCODE_GOOD;
 }