Przeglądaj źródła

squash some coverity bugs

Julius Pfrommer 10 lat temu
rodzic
commit
241d70c00d

+ 9 - 9
examples/networklayer_tcp.c

@@ -136,10 +136,10 @@ static UA_StatusCode NetworkLayerTCP_add(NetworkLayerTCP *layer, UA_Int32 newsoc
 	return UA_STATUSCODE_GOOD;
 }
 
-// Takes the linked list of closed connections and returns the work for the server loop
-static UA_UInt32 batchDeleteLinks(NetworkLayerTCP *layer, UA_WorkItem **returnWork) {
-    UA_WorkItem *work = malloc(sizeof(UA_WorkItem)*layer->conLinksSize);
-	if (!work) {
+/* Removes all connections from the network layer. Returns the work items to close them properly. */
+static UA_UInt32 removeAllConnections(NetworkLayerTCP *layer, UA_WorkItem **returnWork) {
+    UA_WorkItem *work;
+	if (layer->conLinksSize <= 0 || !(work = malloc(sizeof(UA_WorkItem)*layer->conLinksSize))) {
 		*returnWork = NULL;
 		return 0;
 	}
@@ -194,12 +194,12 @@ void closeConnection(TCPConnection *handle) {
 }
 #else
 void closeConnection(TCPConnection *handle) {
+    if(handle->connection.state == UA_CONNECTION_CLOSING)
+        return;
+
 	struct deleteLink *d = malloc(sizeof(struct deleteLink));
 	if(!d)
 		return;
-
-    if(handle->connection.state == UA_CONNECTION_CLOSING)
-        return;
     handle->connection.state = UA_CONNECTION_CLOSING;
 
     UA_Connection_detachSecureChannel(&handle->connection);
@@ -304,7 +304,7 @@ static UA_StatusCode NetworkLayerTCP_start(NetworkLayerTCP *layer, UA_Logger *lo
 static UA_Int32 NetworkLayerTCP_getWork(NetworkLayerTCP *layer, UA_WorkItem **workItems,
                                         UA_UInt16 timeout) {
     UA_WorkItem *items = (void*)0;
-    UA_Int32 itemsCount = batchDeleteLinks(layer, &items);
+    UA_Int32 itemsCount = removeAllConnections(layer, &items);
     setFDSet(layer);
     struct timeval tmptv = {0, timeout};
     UA_Int32 resultsize = select(layer->highestfd+1, &layer->fdset, NULL, NULL, &tmptv);
@@ -375,7 +375,7 @@ static UA_Int32 NetworkLayerTCP_stop(NetworkLayerTCP * layer, UA_WorkItem **work
 #ifdef _WIN32
 	WSACleanup();
 #endif
-    return batchDeleteLinks(layer, workItems);
+    return removeAllConnections(layer, workItems);
 }
 
 static void NetworkLayerTCP_delete(NetworkLayerTCP *layer) {

+ 1 - 1
src/server/ua_nodes.c

@@ -175,7 +175,7 @@ void UA_VariableTypeNode_delete(UA_VariableTypeNode *p) {
 UA_StatusCode UA_VariableTypeNode_copy(const UA_VariableTypeNode *src, UA_VariableTypeNode *dst) {
     UA_VariableTypeNode_init(dst);
 	UA_StatusCode retval = UA_Node_copy((const UA_Node*)src, (UA_Node*)dst);
-    retval = UA_Variant_copy(&src->value, &dst->value);
+    retval |= UA_Variant_copy(&src->value, &dst->value);
     if(retval) {
         UA_VariableTypeNode_deleteMembers(dst);
         return retval;

+ 12 - 10
src/server/ua_securechannel_manager.c

@@ -21,16 +21,18 @@ UA_StatusCode UA_SecureChannelManager_init(UA_SecureChannelManager *cm, UA_UInt3
 }
 
 void UA_SecureChannelManager_deleteMembers(UA_SecureChannelManager *cm) {
-    struct channel_list_entry *entry = LIST_FIRST(&cm->channels);
-    while(entry) {
-        LIST_REMOVE(entry, pointers);
-        if(entry->channel.session)
-            entry->channel.session->channel = UA_NULL;
-        if(entry->channel.connection)
-            entry->channel.connection->channel = UA_NULL;
-        UA_SecureChannel_deleteMembers(&entry->channel);
-        UA_free(entry);
-        entry = LIST_FIRST(&cm->channels);
+    struct channel_list_entry *current;
+    struct channel_list_entry *next = LIST_FIRST(&cm->channels);
+    while(next) {
+        current = next;
+        next = LIST_NEXT(current, pointers);
+        LIST_REMOVE(current, pointers);
+        if(current->channel.session)
+            current->channel.session->channel = UA_NULL;
+        if(current->channel.connection)
+            current->channel.connection->channel = UA_NULL;
+        UA_SecureChannel_deleteMembers(&current->channel);
+        UA_free(current);
     }
 }
 

+ 6 - 7
src/server/ua_server_binary.c

@@ -187,10 +187,8 @@ static void processMSG(UA_Connection *connection, UA_Server *server, const UA_By
 
     //UA_SecureChannel_checkSequenceNumber(channel,sequenceHeader.sequenceNumber);
     //UA_SecureChannel_checkRequestId(channel,sequenceHeader.requestId);
-    if(clientChannel) {
-        clientChannel->sequenceNumber = sequenceHeader.sequenceNumber;
-        clientChannel->requestId = sequenceHeader.requestId;
-    }
+    clientChannel->sequenceNumber = sequenceHeader.sequenceNumber;
+    clientChannel->requestId = sequenceHeader.requestId;
 
     // 3) Read the nodeid of the request
     UA_NodeId requestType;
@@ -320,10 +318,11 @@ static void processMSG(UA_Connection *connection, UA_Server *server, const UA_By
             return;
         UA_ResponseHeader_init(&r);
         init_response_header(&p, &r);
-        if(retval == UA_STATUSCODE_GOOD)
-            r.serviceResult = UA_STATUSCODE_BADSERVICEUNSUPPORTED;
-        else
+        r.serviceResult = UA_STATUSCODE_BADSERVICEUNSUPPORTED;
+#ifdef EXTENSION_STATELESS
+        if(retval != UA_STATUSCODE_GOOD)
             r.serviceResult = retval;
+#endif
         ALLOC_MESSAGE(message, UA_ResponseHeader_calcSizeBinary(&r));
         UA_ResponseHeader_encodeBinary(&r, message, &sendOffset);
         UA_RequestHeader_deleteMembers(&p);

+ 19 - 11
src/server/ua_server_worker.c

@@ -250,7 +250,10 @@ static UA_UInt16 processTimedWork(UA_Server *server) {
             UA_free(tw);
         }
 #else
-        processWork(server,tw->work, tw->workSize); // does not free the work
+        // 1) Process the work since it is past its due date
+        processWork(server, tw->work, tw->workSize); // does not free the work
+
+        // 2) If the work is repeated, add it back into the list. Otherwise remove it.
         if(tw->repetitionInterval > 0) {
             tw->time += tw->repetitionInterval;
             UA_TimedWork *prevTw = tw;
@@ -273,22 +276,27 @@ static UA_UInt16 processTimedWork(UA_Server *server) {
 #endif
     }
 
-    tw = LIST_FIRST(&server->timedWork);
+    // check if the next timed work is sooner than the usual timeout
+    UA_TimedWork *first = LIST_FIRST(&server->timedWork);
     UA_UInt16 timeout = MAXTIMEOUT;
-    if(tw){
-        timeout = (tw->time - current)/10;
-        if(timeout>MAXTIMEOUT)timeout = MAXTIMEOUT;
+    if(first) {
+        timeout = (first->time - current)/10;
+        if(timeout > MAXTIMEOUT)
+            timeout = MAXTIMEOUT;
     }
     return timeout;
 }
 
 void UA_Server_deleteTimedWork(UA_Server *server) {
-    UA_TimedWork *tw;
-    while((tw = LIST_FIRST(&server->timedWork))) {
-        LIST_REMOVE(tw, pointers);
-        UA_free(tw->work);
-        UA_free(tw->workIds);
-        UA_free(tw);
+    UA_TimedWork *current;
+    UA_TimedWork *next = LIST_FIRST(&server->timedWork);
+    while(next) {
+        current = next;
+        next = LIST_NEXT(current, pointers);
+        LIST_REMOVE(current, pointers);
+        UA_free(current->work);
+        UA_free(current->workIds);
+        UA_free(current);
     }
 }
 

+ 5 - 4
src/server/ua_services_view.c

@@ -170,12 +170,13 @@ static void getBrowseResult(UA_NodeStore *ns, const UA_BrowseDescription *browse
         return;
     }
 
-    maxReferences = parentNode->referencesSize;
-    // 0 => unlimited references
+    maxReferences = parentNode->referencesSize; // 0 => unlimited references
     if(maxReferences <= 0 || maxReferences > UA_INT32_MAX ||
        (UA_Int32)maxReferences > parentNode->referencesSize) {
-        if(parentNode->referencesSize < 0)
-            maxReferences = 0;
+        if(parentNode->referencesSize <= 0) {
+            browseResult->referencesSize = 0;
+            return;
+        }
         else
             maxReferences = parentNode->referencesSize;
     }

+ 5 - 3
src/server/ua_session_manager.c

@@ -24,14 +24,16 @@ UA_StatusCode UA_SessionManager_init(UA_SessionManager *sessionManager, UA_UInt3
 }
 
 void UA_SessionManager_deleteMembers(UA_SessionManager *sessionManager) {
-    struct session_list_entry *current = LIST_FIRST(&sessionManager->sessions);
-    while(current) {
+    struct session_list_entry *current;
+    struct session_list_entry *next = LIST_FIRST(&sessionManager->sessions);
+    while(next) {
+        current = next;
+        next = LIST_NEXT(current, pointers);
         LIST_REMOVE(current, pointers);
         if(current->session.channel)
             current->session.channel->session = UA_NULL; // the channel is no longer attached to a session
         UA_Session_deleteMembers(&current->session);
         UA_free(current);
-        current = LIST_FIRST(&sessionManager->sessions);
     }
 }
 

+ 25 - 8
src/ua_types.c

@@ -664,7 +664,13 @@ void UA_DiagnosticInfo_deleteMembers(UA_DiagnosticInfo *p) {
 
 UA_TYPE_NEW_DEFAULT(UA_DiagnosticInfo)
 void UA_DiagnosticInfo_init(UA_DiagnosticInfo *p) {
-    *(UA_Byte*)p = 0;
+    p->hasSymbolicId          = UA_FALSE;
+    p->hasNamespaceUri        = UA_FALSE;
+    p->hasLocalizedText       = UA_FALSE;
+    p->hasLocale              = UA_FALSE;
+    p->hasAdditionalInfo      = UA_FALSE;
+    p->hasInnerStatusCode     = UA_FALSE;
+    p->hasInnerDiagnosticInfo = UA_FALSE;
     p->symbolicId          = 0;
     p->namespaceUri        = 0;
     p->localizedText       = 0;
@@ -676,19 +682,30 @@ void UA_DiagnosticInfo_init(UA_DiagnosticInfo *p) {
 
 UA_StatusCode UA_DiagnosticInfo_copy(UA_DiagnosticInfo const *src, UA_DiagnosticInfo *dst) {
     UA_DiagnosticInfo_init(dst);
-    *(UA_Byte*)dst = *(const UA_Byte*)src; // bitfields
-    UA_StatusCode retval = UA_String_copy(&src->additionalInfo, &dst->additionalInfo);
-    retval |= UA_StatusCode_copy(&src->innerStatusCode, &dst->innerStatusCode);
+    dst->hasSymbolicId          = src->hasSymbolicId;
+    dst->hasNamespaceUri        = src->hasNamespaceUri;
+    dst->hasLocalizedText       = src->hasLocalizedText;
+    dst->hasLocale              = src->hasLocale;
+    dst->hasAdditionalInfo      = src->hasAdditionalInfo;
+    dst->hasInnerStatusCode     = src->hasInnerStatusCode;
+    dst->hasInnerDiagnosticInfo = src->hasInnerDiagnosticInfo;
+    
+    dst->symbolicId = src->symbolicId;
+    dst->namespaceUri = src->namespaceUri;
+    dst->localizedText = src->localizedText;
+    dst->locale = src->locale;
+    dst->innerStatusCode = src->innerStatusCode;
+    UA_StatusCode retval = UA_STATUSCODE_GOOD;
+    if(src->hasAdditionalInfo)
+       retval = UA_String_copy(&src->additionalInfo, &dst->additionalInfo);
     if(src->hasInnerDiagnosticInfo && src->innerDiagnosticInfo) {
         if((dst->innerDiagnosticInfo = UA_malloc(sizeof(UA_DiagnosticInfo))))
             retval |= UA_DiagnosticInfo_copy(src->innerDiagnosticInfo, dst->innerDiagnosticInfo);
         else
             retval |= UA_STATUSCODE_BADOUTOFMEMORY;
+    } else {
+        dst->hasInnerDiagnosticInfo = UA_FALSE;
     }
-    dst->locale = src->locale;
-    dst->localizedText = src->localizedText;
-    dst->namespaceUri = src->namespaceUri;
-    dst->symbolicId = src->symbolicId;
     if(retval) {
         UA_DiagnosticInfo_deleteMembers(dst);
         UA_DiagnosticInfo_init(dst);