some more sometic cleanups. use list macros when possible

src/server/ua_services_subscription.c

@@ -7,19 +7,16 @@
 #include "ua_nodestore.h"
 
 #define UA_BOUNDEDVALUE_SETWBOUNDS(BOUNDS, SRC, DST) { \
-    if (SRC > BOUNDS.maxValue) DST = BOUNDS.maxValue; \
-    else if (SRC < BOUNDS.minValue) DST = BOUNDS.minValue; \
+    if(SRC > BOUNDS.maxValue) DST = BOUNDS.maxValue; \
+    else if(SRC < BOUNDS.minValue) DST = BOUNDS.minValue; \
     else DST = SRC; \
     }
 
 void Service_CreateSubscription(UA_Server *server, UA_Session *session,
                                 const UA_CreateSubscriptionRequest *request,
                                 UA_CreateSubscriptionResponse *response) {
-    UA_Subscription *newSubscription;
-    
-    // Create Subscription and Response
-    response->subscriptionId = SubscriptionManager_getUniqueUIntID(&(session->subscriptionManager));
-    newSubscription = UA_Subscription_new(response->subscriptionId);
+    response->subscriptionId = SubscriptionManager_getUniqueUIntID(&session->subscriptionManager);
+    UA_Subscription *newSubscription = UA_Subscription_new(response->subscriptionId);
     
     UA_BOUNDEDVALUE_SETWBOUNDS(session->subscriptionManager.GlobalPublishingInterval,
                                request->requestedPublishingInterval, response->revisedPublishingInterval);
@@ -43,90 +40,79 @@ void Service_CreateSubscription(UA_Server *server, UA_Session *session,
     newSubscription->PublishingMode          = request->publishingEnabled;
     newSubscription->Priority                = request->priority;
     
-    UA_Guid jobId = SubscriptionManager_getUniqueGUID(&(session->subscriptionManager));
+    UA_Guid jobId = SubscriptionManager_getUniqueGUID(&session->subscriptionManager);
     Subscription_createdUpdateJob(server, jobId, newSubscription);
     Subscription_registerUpdateJob(server, newSubscription);
-    SubscriptionManager_addSubscription(&(session->subscriptionManager), newSubscription);    
+    SubscriptionManager_addSubscription(&session->subscriptionManager, newSubscription);    
+}
+
+static void createMonitoredItems(UA_Server *server, UA_Session *session,
+                                 UA_Subscription *sub, const UA_MonitoredItemCreateRequest *request,
+                                 UA_MonitoredItemCreateResult *result) {
+    const UA_Node *target = UA_NodeStore_get(server->nodestore, &request->itemToMonitor.nodeId);
+    if(!target) {
+        result->statusCode = UA_STATUSCODE_BADNODEIDINVALID;
+        result->monitoredItemId = 0;
+        result->revisedSamplingInterval = 0;
+        result->revisedQueueSize = 0;
+        return;
+    }
+
+    UA_MonitoredItem *newMon = UA_MonitoredItem_new();
+    UA_NodeId_copy(&target->nodeId, &newMon->monitoredNodeId);
+    newMon->ItemId = ++(session->subscriptionManager.LastSessionID);
+    result->monitoredItemId = newMon->ItemId;
+    newMon->ClientHandle = request->requestedParameters.clientHandle;
+    UA_BOUNDEDVALUE_SETWBOUNDS(session->subscriptionManager.GlobalSamplingInterval,
+                               request->requestedParameters.samplingInterval,
+                               result->revisedSamplingInterval);
+    newMon->SamplingInterval = result->revisedSamplingInterval;
+    UA_BOUNDEDVALUE_SETWBOUNDS(session->subscriptionManager.GlobalQueueSize,
+                               request->requestedParameters.queueSize,
+                               result->revisedQueueSize);
+    newMon->QueueSize = (UA_UInt32_BoundedValue) {
+        .maxValue=(result->revisedQueueSize) + 1,
+        .minValue=0, .currentValue=0 };
+    newMon->AttributeID = request->itemToMonitor.attributeId;
+    newMon->MonitoredItemType = MONITOREDITEM_TYPE_CHANGENOTIFY;
+    newMon->DiscardOldest = request->requestedParameters.discardOldest;
+    LIST_INSERT_HEAD(&sub->MonitoredItems, newMon, listEntry);
+    UA_NodeStore_release(target);
 }
 
 void Service_CreateMonitoredItems(UA_Server *server, UA_Session *session,
                                   const UA_CreateMonitoredItemsRequest *request,
                                   UA_CreateMonitoredItemsResponse *response) {
-    UA_MonitoredItem *newMon;
-    UA_Subscription  *sub;
-    UA_MonitoredItemCreateResult *createResults, *thisItemsResult;
-    UA_MonitoredItemCreateRequest *thisItemsRequest;
-    
-    // Verify Session and Subscription
-    sub = SubscriptionManager_getSubscriptionByID(&(session->subscriptionManager), request->subscriptionId);
+    UA_Subscription  *sub = SubscriptionManager_getSubscriptionByID(&session->subscriptionManager,
+                                                                    request->subscriptionId);
     if(!sub) {
         response->responseHeader.serviceResult = UA_STATUSCODE_BADSUBSCRIPTIONIDINVALID;
         return;
     }
     
-    // Allocate Result Array
-    if (request->itemsToCreateSize <= 0)
+    if(request->itemsToCreateSize <= 0) {
+        response->responseHeader.serviceResult = UA_STATUSCODE_BADNOTHINGTODO;
         return;
+    }
 
-    createResults = UA_malloc(sizeof(UA_MonitoredItemCreateResult) * (request->itemsToCreateSize));
-    if(!createResults) {
+    response->results = UA_Array_new(&UA_TYPES[UA_TYPES_MONITOREDITEMCREATERESULT], request->itemsToCreateSize);
+    if(!response->results) {
         response->responseHeader.serviceResult = UA_STATUSCODE_BADOUTOFMEMORY;
         return;
     }
-    
     response->resultsSize = request->itemsToCreateSize;
-    response->results = createResults;
-    
-    for(int i=0; i<request->itemsToCreateSize; i++) {
-        thisItemsRequest = &request->itemsToCreate[i];
-        thisItemsResult  = &createResults[i];
-        
-        // FIXME: Completely ignoring filters for now!
-        thisItemsResult->filterResult.typeId   = UA_NODEID_NULL;
-        thisItemsResult->filterResult.encoding = UA_EXTENSIONOBJECT_ENCODINGMASK_NOBODYISENCODED;
-        thisItemsResult->filterResult.body     = UA_BYTESTRING_NULL;
-        
-        const UA_Node *target = UA_NodeStore_get(server->nodestore, &thisItemsRequest->itemToMonitor.nodeId);
-        if(!target) {
-            thisItemsResult->statusCode = UA_STATUSCODE_BADNODEIDINVALID;
-            thisItemsResult->monitoredItemId = 0;
-            thisItemsResult->revisedSamplingInterval = 0;
-            thisItemsResult->revisedQueueSize = 0;
-            return;
-        }
 
-        thisItemsResult->statusCode = UA_STATUSCODE_GOOD;
-        newMon = UA_MonitoredItem_new();
-        UA_NodeId_copy(&target->nodeId, &newMon->monitoredNodeId);
-        newMon->ItemId = ++(session->subscriptionManager.LastSessionID);
-        thisItemsResult->monitoredItemId = newMon->ItemId;
-        newMon->ClientHandle = thisItemsRequest->requestedParameters.clientHandle;
-        UA_BOUNDEDVALUE_SETWBOUNDS(session->subscriptionManager.GlobalSamplingInterval,
-                                   thisItemsRequest->requestedParameters.samplingInterval,
-                                   thisItemsResult->revisedSamplingInterval);
-        newMon->SamplingInterval = thisItemsResult->revisedSamplingInterval;
-        UA_BOUNDEDVALUE_SETWBOUNDS(session->subscriptionManager.GlobalQueueSize,
-                                   thisItemsRequest->requestedParameters.queueSize,
-                                   thisItemsResult->revisedQueueSize);
-        newMon->QueueSize = (UA_UInt32_BoundedValue) {
-            .maxValue=(thisItemsResult->revisedQueueSize) + 1, .minValue=0, .currentValue=0 };
-        newMon->AttributeID = thisItemsRequest->itemToMonitor.attributeId;
-        newMon->MonitoredItemType = MONITOREDITEM_TYPE_CHANGENOTIFY;
-        newMon->DiscardOldest = thisItemsRequest->requestedParameters.discardOldest;
-        LIST_INSERT_HEAD(&sub->MonitoredItems, newMon, listEntry);
-        UA_NodeStore_release(target);
-    }
+    for(UA_Int32 i = 0; i<request->itemsToCreateSize; i++)
+        createMonitoredItems(server, session, sub, &request->itemsToCreate[i], &response->results[i]);
+}
+
+static void publish(UA_Server *server, UA_Session *session, UA_Subscription *sub,
+                    const UA_PublishRequest *request, UA_PublishResponse *response) {
+
 }
 
 void Service_Publish(UA_Server *server, UA_Session *session, const UA_PublishRequest *request,
                      UA_PublishResponse *response) {
-    UA_Subscription  *sub;
-    UA_MonitoredItem *mon;
-    
-    UA_PublishResponse_init(response);
-    // Verify Session and Subscription
-    response->responseHeader.serviceResult = UA_STATUSCODE_GOOD;
-        
     UA_SubscriptionManager *manager= &session->subscriptionManager;
     if(!manager)
         return;
@@ -134,23 +120,27 @@ void Service_Publish(UA_Server *server, UA_Session *session, const UA_PublishReq
     // Delete Acknowledged Subscription Messages
     response->resultsSize = request->subscriptionAcknowledgementsSize;
     response->results     = UA_malloc(sizeof(UA_StatusCode)*(response->resultsSize));
-    for(int i=0; i<request->subscriptionAcknowledgementsSize;i++ ) {
+    for(UA_Int32 i = 0; i < request->subscriptionAcknowledgementsSize; i++) {
         response->results[i] = UA_STATUSCODE_GOOD;
-        sub = SubscriptionManager_getSubscriptionByID(&session->subscriptionManager,
-                                                      request->subscriptionAcknowledgements[i].subscriptionId);
+        UA_Subscription *sub =
+            SubscriptionManager_getSubscriptionByID(&session->subscriptionManager,
+                                                    request->subscriptionAcknowledgements[i].subscriptionId);
         if(!sub) {
             response->results[i] = UA_STATUSCODE_BADSUBSCRIPTIONIDINVALID;
             continue;
         }
-        if(Subscription_deleteUnpublishedNotification(request->subscriptionAcknowledgements[i].sequenceNumber, sub) == 0)
+        if(Subscription_deleteUnpublishedNotification(request->subscriptionAcknowledgements[i].sequenceNumber,
+                                                      sub) == 0)
             response->results[i] = UA_STATUSCODE_BADSEQUENCENUMBERINVALID;
     }
     
     // See if any new data is available
+    UA_Subscription *sub;
     LIST_FOREACH(sub, &manager->ServerSubscriptions, listEntry) {
-        if (sub->timedUpdateIsRegistered == UA_FALSE) {
+        if(sub->timedUpdateIsRegistered == UA_FALSE) {
             // FIXME: We are forcing a value update for monitored items. This should be done by the event system.
             // NOTE:  There is a clone of this functionality in the Subscription_timedUpdateNotificationsJob
+            UA_MonitoredItem *mon;
             LIST_FOREACH(mon, &sub->MonitoredItems, listEntry)
                 MonitoredItem_QueuePushDataValue(server, mon);
             
@@ -163,8 +153,8 @@ void Service_Publish(UA_Server *server, UA_Session *session, const UA_PublishReq
             continue;
         
         response->subscriptionId = sub->SubscriptionID;
-        Subscription_copyTopNotificationMessage(&(response->notificationMessage), sub);
-        if (sub->unpublishedNotifications.lh_first->notification->sequenceNumber > sub->SequenceNumber) {
+        Subscription_copyTopNotificationMessage(&response->notificationMessage, sub);
+        if(sub->unpublishedNotifications.lh_first->notification->sequenceNumber > sub->SequenceNumber) {
             // If this is a keepalive message, its seqNo is the next seqNo to be used for an actual msg.
             response->availableSequenceNumbersSize = 0;
             // .. and must be deleted
@@ -182,36 +172,29 @@ void Service_Publish(UA_Server *server, UA_Session *session, const UA_PublishReq
     // request, but currently we need to return something to the client. If no
     // subscriptions have notifications, force one to generate a keepalive so we
     // don't return an empty message
-    sub = manager->ServerSubscriptions.lh_first;
+    sub = LIST_FIRST(&manager->ServerSubscriptions);
     if(!sub) {
         response->subscriptionId = sub->SubscriptionID;
         sub->KeepAliveCount.currentValue=sub->KeepAliveCount.minValue;
         Subscription_generateKeepAlive(sub);
-        Subscription_copyTopNotificationMessage(&(response->notificationMessage), sub);
+        Subscription_copyTopNotificationMessage(&response->notificationMessage, sub);
         Subscription_deleteUnpublishedNotification(sub->SequenceNumber + 1, sub);
-        response->availableSequenceNumbersSize = 0;
     }
     
     // FIXME: This should be in processMSG();
     session->validTill = UA_DateTime_now() + session->timeout * 10000;
-    response->diagnosticInfosSize = 0;
-    response->diagnosticInfos     = 0;
 }
 
 void Service_ModifySubscription(UA_Server *server, UA_Session *session,
                                  const UA_ModifySubscriptionRequest *request,
                                  UA_ModifySubscriptionResponse *response) {
-    UA_Subscription *sub;
-    
-    response->responseHeader.serviceResult = UA_STATUSCODE_GOOD;
-    
-    sub = SubscriptionManager_getSubscriptionByID(&(session->subscriptionManager), request->subscriptionId);
-    if (!sub) {
+    UA_Subscription *sub = SubscriptionManager_getSubscriptionByID(&session->subscriptionManager,
+                                                                   request->subscriptionId);
+    if(!sub) {
         response->responseHeader.serviceResult = UA_STATUSCODE_BADSUBSCRIPTIONIDINVALID;
         return;
     }
     
-    
     UA_BOUNDEDVALUE_SETWBOUNDS(session->subscriptionManager.GlobalPublishingInterval,
                                request->requestedPublishingInterval, response->revisedPublishingInterval);
     sub->PublishingInterval = response->revisedPublishingInterval;
@@ -233,9 +216,7 @@ void Service_ModifySubscription(UA_Server *server, UA_Session *session,
     sub->NotificationsPerPublish = request->maxNotificationsPerPublish;
     sub->Priority                = request->priority;
     
-    printf("Unregister returned: %u\n", Subscription_unregisterUpdateJob(server, sub));
     Subscription_registerUpdateJob(server, sub);
-    printf("Modified\n");
     return;
 }
 
@@ -243,23 +224,21 @@ void Service_DeleteSubscriptions(UA_Server *server, UA_Session *session,
                                  const UA_DeleteSubscriptionsRequest *request,
                                  UA_DeleteSubscriptionsResponse *response) {
     response->results = UA_malloc(sizeof(UA_StatusCode) * request->subscriptionIdsSize);
-    if (!response->results) {
+    if(!response->results) {
         response->responseHeader.serviceResult = UA_STATUSCODE_BADOUTOFMEMORY;
         return;
     }
     response->resultsSize = request->subscriptionIdsSize;
-    for(int i=0; i<request->subscriptionIdsSize;i++)
-        response->results[i] = SubscriptionManager_deleteSubscription(server, &(session->subscriptionManager),
+
+    for(UA_Int32 i = 0; i < request->subscriptionIdsSize; i++)
+        response->results[i] = SubscriptionManager_deleteSubscription(server, &session->subscriptionManager,
                                                                       request->subscriptionIds[i]);
 } 
 
 void Service_DeleteMonitoredItems(UA_Server *server, UA_Session *session,
                                   const UA_DeleteMonitoredItemsRequest *request,
                                   UA_DeleteMonitoredItemsResponse *response) {
-    response->diagnosticInfosSize=0;
-    response->resultsSize=0;
-    
-    UA_SubscriptionManager *manager = &(session->subscriptionManager);
+    UA_SubscriptionManager *manager = &session->subscriptionManager;
     UA_Subscription *sub = SubscriptionManager_getSubscriptionByID(manager, request->subscriptionId);
     if(!sub) {
         response->responseHeader.serviceResult = UA_STATUSCODE_BADSUBSCRIPTIONIDINVALID;
@@ -267,13 +246,13 @@ void Service_DeleteMonitoredItems(UA_Server *server, UA_Session *session,
     }
     
     response->results = UA_malloc(sizeof(UA_StatusCode) * request->monitoredItemIdsSize);
-    if (!response->results) {
+    if(!response->results) {
         response->responseHeader.serviceResult = UA_STATUSCODE_BADOUTOFMEMORY;
         return;
     }
-
     response->resultsSize = request->monitoredItemIdsSize;
-    for(int i=0; i < request->monitoredItemIdsSize; i++)
+
+    for(UA_Int32 i = 0; i < request->monitoredItemIdsSize; i++)
         response->results[i] = SubscriptionManager_deleteMonitoredItem(manager, sub->SubscriptionID,
-                                                                 request->monitoredItemIds[i]);
+                                                                       request->monitoredItemIds[i]);
 }

src/server/ua_subscription.c

@@ -30,15 +30,13 @@ void UA_Subscription_deleteMembers(UA_Subscription *subscription, UA_Server *ser
     subscription->SubscriptionID = 0;
     
     // Delete monitored Items
-    while(subscription->MonitoredItems.lh_first != NULL) {
-        mon = subscription->MonitoredItems.lh_first;
+    while((mon = LIST_FIRST(&subscription->MonitoredItems))) {
         LIST_REMOVE(mon, listEntry);
         MonitoredItem_delete(mon);
     }
     
     // Delete unpublished Notifications
-    while(subscription->unpublishedNotifications.lh_first !=  NULL) {
-        not = subscription->unpublishedNotifications.lh_first;
+    while((not = LIST_FIRST(&subscription->unpublishedNotifications))) {
         LIST_REMOVE(not, listEntry);
         Subscription_deleteUnpublishedNotification(not->notification->sequenceNumber, subscription);
     }
@@ -217,7 +215,7 @@ void Subscription_copyTopNotificationMessage(UA_NotificationMessage *dst, UA_Sub
       return;
     }
     
-    latest = sub->unpublishedNotifications.lh_first->notification;
+    latest = LIST_FIRST(&sub->unpublishedNotifications)->notification;
     dst->notificationDataSize = latest->notificationDataSize;
     dst->publishTime = latest->publishTime;
     dst->sequenceNumber = latest->sequenceNumber;
@@ -232,23 +230,20 @@ void Subscription_copyTopNotificationMessage(UA_NotificationMessage *dst, UA_Sub
 }
 
 UA_UInt32 Subscription_deleteUnpublishedNotification(UA_UInt32 seqNo, UA_Subscription *sub) {
-    UA_unpublishedNotification *not, *nxtnot = UA_NULL;
     UA_UInt32 deletedItems = 0;
-  
-    for(not=sub->unpublishedNotifications.lh_first; not != NULL; not=nxtnot) {
-        nxtnot = not->listEntry.le_next;
+    UA_unpublishedNotification *not, *tmp;
+    LIST_FOREACH_SAFE(not, &sub->unpublishedNotifications, listEntry, tmp) {
         if(not->notification->sequenceNumber != seqNo)
             continue;
         LIST_REMOVE(not, listEntry);
-        if(not->notification != NULL) {
-            if(not->notification->notificationData != NULL) {
-                if(not->notification->notificationData->body.data != NULL)
+        if(not->notification) {
+            if(not->notification->notificationData) {
+                if(not->notification->notificationData->body.data)
                     UA_free(not->notification->notificationData->body.data);
                 UA_free(not->notification->notificationData);
             }
             UA_free(not->notification);
         }
-        
         UA_free(not);
         deletedItems++;
     }
@@ -298,7 +293,6 @@ UA_StatusCode Subscription_createdUpdateJob(UA_Server *server, UA_Guid jobId, UA
 }
 
 UA_StatusCode Subscription_registerUpdateJob(UA_Server *server, UA_Subscription *sub) {
-    UA_Int32 retval = UA_STATUSCODE_GOOD;
     if(server == UA_NULL || sub == UA_NULL)
         return UA_STATUSCODE_BADSERVERINDEXINVALID;
     
@@ -307,11 +301,11 @@ UA_StatusCode Subscription_registerUpdateJob(UA_Server *server, UA_Subscription
     
     // Practically enough, the client sends a uint32 in ms, which we store as datetime, which here is required in as uint32 in ms as the interval
 #ifdef _MSC_VER
-    retval |= UA_Server_addRepeatedJob(server, *(sub->timedUpdateJob), sub->PublishingInterval,
-                                       &sub->timedUpdateJobGuid);
+    UA_Int32 retval = UA_Server_addRepeatedJob(server, *(sub->timedUpdateJob), sub->PublishingInterval,
+                                               &sub->timedUpdateJobGuid);
 #else
-    retval |= UA_Server_addRepeatedJob(server, *sub->timedUpdateJob, sub->PublishingInterval,
-                                       &sub->timedUpdateJobGuid);
+    UA_StatusCode retval = UA_Server_addRepeatedJob(server, *sub->timedUpdateJob, sub->PublishingInterval,
+                                                    &sub->timedUpdateJobGuid);
 #endif
     if(!retval)
         sub->timedUpdateIsRegistered = UA_TRUE;
@@ -319,16 +313,13 @@ UA_StatusCode Subscription_registerUpdateJob(UA_Server *server, UA_Subscription
 }
 
 UA_StatusCode Subscription_unregisterUpdateJob(UA_Server *server, UA_Subscription *sub) {
-    UA_Int32 retval = UA_STATUSCODE_GOOD;
     if(server == UA_NULL || sub == UA_NULL)
         return UA_STATUSCODE_BADSERVERINDEXINVALID;
-    
-    retval |= UA_Server_removeRepeatedJob(server, sub->timedUpdateJobGuid);
+    UA_Int32 retval = UA_Server_removeRepeatedJob(server, sub->timedUpdateJobGuid);
     sub->timedUpdateIsRegistered = UA_FALSE;
     return retval;
 }
 
-
 /*****************/
 /* MonitoredItem */
 /*****************/