Explorar el Código

remove bugs found with clang analyzer

Julius Pfrommer hace 9 años
padre
commit
5bc8d06cb0

+ 2 - 1
include/ua_types.h

@@ -541,7 +541,8 @@ void UA_EXPORT * UA_new(const UA_DataType *type) UA_FUNC_ATTR_MALLOC;
  * @param p The memory location of the variable
  * @param type The datatype description
  */
-void UA_EXPORT UA_init(void *p, const UA_DataType *type);
+static UA_INLINE void UA_init(void *p, const UA_DataType *type) {
+    memset(p, 0, type->memSize); }
 
 /**
  * Copies the content of two variables. If copying fails (e.g. because no memory was available for

+ 6 - 2
src/client/ua_client_highlevel_subscriptions.c

@@ -282,8 +282,12 @@ void UA_Client_Subscriptions_manuallySendPublishRequest(UA_Client *client) {
         UA_Client_NotificationsAckNumber *ack;
         LIST_FOREACH(ack, &client->pendingNotificationsAcks, listEntry)
             request.subscriptionAcknowledgementsSize++;
-        request.subscriptionAcknowledgements = UA_malloc(sizeof(UA_SubscriptionAcknowledgement) *
-                                                         request.subscriptionAcknowledgementsSize);
+        if(request.subscriptionAcknowledgementsSize > 0) {
+            request.subscriptionAcknowledgements = UA_malloc(sizeof(UA_SubscriptionAcknowledgement) *
+                                                             request.subscriptionAcknowledgementsSize);
+            if(!request.subscriptionAcknowledgements)
+                return;
+        }
         
         int index = 0 ;
         LIST_FOREACH(ack, &client->pendingNotificationsAcks, listEntry) {

+ 8 - 13
src/server/ua_securechannel_manager.c

@@ -49,22 +49,17 @@ void UA_SecureChannelManager_cleanupTimedOut(UA_SecureChannelManager *cm,
     }
 }
 
-UA_StatusCode UA_SecureChannelManager_open(UA_SecureChannelManager *cm,
-        UA_Connection *conn, const UA_OpenSecureChannelRequest *request,
-        UA_OpenSecureChannelResponse *response) {
-
-    if (request->securityMode != UA_MESSAGESECURITYMODE_NONE) {
+UA_StatusCode
+UA_SecureChannelManager_open(UA_SecureChannelManager *cm, UA_Connection *conn,
+                             const UA_OpenSecureChannelRequest *request,
+                             UA_OpenSecureChannelResponse *response) {
+    if(request->securityMode != UA_MESSAGESECURITYMODE_NONE)
         return UA_STATUSCODE_BADSECURITYMODEREJECTED;
-
-    }
-
-    channel_list_entry *entry = UA_malloc(sizeof(channel_list_entry));
-    if (!entry) {
+    if(cm->currentChannelCount >= cm->maxChannelCount)
         return UA_STATUSCODE_BADOUTOFMEMORY;
-    }
-    if (cm->currentChannelCount >= cm->maxChannelCount) {
+    channel_list_entry *entry = UA_malloc(sizeof(channel_list_entry));
+    if (!entry)
         return UA_STATUSCODE_BADOUTOFMEMORY;
-    }
 #ifndef UA_MULTITHREADING
     cm->currentChannelCount++;
 #else

+ 2 - 1
src/server/ua_services_subscription.c

@@ -172,7 +172,8 @@ void Service_Publish(UA_Server *server, UA_Session *session, const UA_PublishReq
         
         response->subscriptionId = sub->subscriptionID;
         Subscription_copyTopNotificationMessage(&response->notificationMessage, sub);
-        if(sub->unpublishedNotifications.lh_first->notification->sequenceNumber > sub->sequenceNumber) {
+        UA_unpublishedNotification *firstUnPublished = LIST_FIRST(&sub->unpublishedNotifications);
+        if(firstUnPublished->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

+ 20 - 24
src/server/ua_subscription.c

@@ -59,16 +59,14 @@ void Subscription_generateKeepAlive(UA_Subscription *subscription) {
        subscription->keepAliveCount.currentValue <= subscription->keepAliveCount.maxValue)
         return;
 
-    UA_unpublishedNotification *msg = UA_malloc(sizeof(UA_unpublishedNotification));
+    UA_unpublishedNotification *msg = UA_calloc(1,sizeof(UA_unpublishedNotification));
     if(!msg)
         return;
-    msg->notification = NULL;
-    msg->notification = UA_malloc(sizeof(UA_NotificationMessage));
-    msg->notification->notificationData = NULL;
+    msg->notification.notificationData = NULL;
     // KeepAlive uses next message, but does not increment counter
-    msg->notification->sequenceNumber = subscription->sequenceNumber + 1;
-    msg->notification->publishTime    = UA_DateTime_now();
-    msg->notification->notificationDataSize = 0;
+    msg->notification.sequenceNumber = subscription->sequenceNumber + 1;
+    msg->notification.publishTime    = UA_DateTime_now();
+    msg->notification.notificationDataSize = 0;
     LIST_INSERT_HEAD(&subscription->unpublishedNotifications, msg, listEntry);
     subscription->keepAliveCount.currentValue = subscription->keepAliveCount.maxValue;
 }
@@ -112,19 +110,18 @@ void Subscription_updateNotifications(UA_Subscription *subscription) {
         return;
     }
     
-    msg = UA_malloc(sizeof(UA_unpublishedNotification));
-    msg->notification = UA_NotificationMessage_new();
-    msg->notification->sequenceNumber = subscription->sequenceNumber++;
-    msg->notification->publishTime    = UA_DateTime_now();
+    msg = UA_calloc(1, sizeof(UA_unpublishedNotification));
+    msg->notification.sequenceNumber = subscription->sequenceNumber++;
+    msg->notification.publishTime    = UA_DateTime_now();
     
     // NotificationData is an array of Change, Status and Event messages, each containing the appropriate
     // list of Queued values from all monitoredItems of that type
-    msg->notification->notificationDataSize = ISNOTZERO(monItemsChangeT);
+    msg->notification.notificationDataSize = !!monItemsChangeT; // 1 if the pointer is not null, else 0
     // + ISNOTZERO(monItemsEventT) + ISNOTZERO(monItemsStatusT);
-    msg->notification->notificationData =
-        UA_Array_new(msg->notification->notificationDataSize, &UA_TYPES[UA_TYPES_EXTENSIONOBJECT]);
+    msg->notification.notificationData =
+        UA_Array_new(msg->notification.notificationDataSize, &UA_TYPES[UA_TYPES_EXTENSIONOBJECT]);
     
-    for(size_t notmsgn = 0; notmsgn < msg->notification->notificationDataSize; notmsgn++) {
+    for(size_t notmsgn = 0; notmsgn < msg->notification.notificationDataSize; notmsgn++) {
         // Set the notification message type and encoding for each of 
         //   the three possible NotificationData Types
         /* msg->notification->notificationData[notmsgn].encoding = 1; // Encoding is always binary */
@@ -145,9 +142,9 @@ void Subscription_updateNotifications(UA_Subscription *subscription) {
                 MonitoredItem_ClearQueue(mon);
             }
             changeNotification->monitoredItemsSize = monItemsChangeT;
-            msg->notification->notificationData[notmsgn].encoding = UA_EXTENSIONOBJECT_DECODED;
-            msg->notification->notificationData[notmsgn].content.decoded.type = &UA_TYPES[UA_TYPES_DATACHANGENOTIFICATION];
-            msg->notification->notificationData[notmsgn].content.decoded.data = changeNotification;
+            msg->notification.notificationData[notmsgn].encoding = UA_EXTENSIONOBJECT_DECODED;
+            msg->notification.notificationData[notmsgn].content.decoded.type = &UA_TYPES[UA_TYPES_DATACHANGENOTIFICATION];
+            msg->notification.notificationData[notmsgn].content.decoded.data = changeNotification;
         } else if(notmsgn == 1) {
             // FIXME: Constructing a StatusChangeNotification is not implemented
         } else if(notmsgn == 2) {
@@ -165,7 +162,7 @@ UA_UInt32 *Subscription_getAvailableSequenceNumbers(UA_Subscription *sub) {
     int i = 0;
     UA_unpublishedNotification *not;
     LIST_FOREACH(not, &sub->unpublishedNotifications, listEntry) {
-        seqArray[i] = not->notification->sequenceNumber;
+        seqArray[i] = not->notification.sequenceNumber;
         i++;
     }
     return seqArray;
@@ -182,7 +179,7 @@ void Subscription_copyTopNotificationMessage(UA_NotificationMessage *dst, UA_Sub
       return;
     }
     
-    UA_NotificationMessage *latest = LIST_FIRST(&sub->unpublishedNotifications)->notification;
+    UA_NotificationMessage *latest = &LIST_FIRST(&sub->unpublishedNotifications)->notification;
     dst->notificationDataSize = latest->notificationDataSize;
     dst->publishTime = latest->publishTime;
     dst->sequenceNumber = latest->sequenceNumber;
@@ -197,11 +194,10 @@ UA_UInt32 Subscription_deleteUnpublishedNotification(UA_UInt32 seqNo, UA_Boolean
     UA_UInt32 deletedItems = 0;
     UA_unpublishedNotification *not, *tmp;
     LIST_FOREACH_SAFE(not, &sub->unpublishedNotifications, listEntry, tmp) {
-        if (!bDeleteAll && not->notification->sequenceNumber != seqNo)
+        if(!bDeleteAll && not->notification.sequenceNumber != seqNo)
             continue;
         LIST_REMOVE(not, listEntry);
-        if(not->notification)
-            UA_NotificationMessage_delete(not->notification);
+        UA_NotificationMessage_deleteMembers(&not->notification);
         UA_free(not);
         deletedItems++;
     }
@@ -283,7 +279,7 @@ UA_MonitoredItem *UA_MonitoredItem_new() {
     new->monitoredItemType = MONITOREDITEM_TYPE_CHANGENOTIFY;
     TAILQ_INIT(&new->queue);
     UA_NodeId_init(&new->monitoredNodeId);
-    INITPOINTER(new->lastSampledValue.data );
+    new->lastSampledValue.data = 0;
     return new;
 }
 

+ 1 - 4
src/server/ua_subscription.h

@@ -6,9 +6,6 @@
 #include "ua_types_generated.h"
 #include "ua_nodes.h"
 
-#define INITPOINTER(src) (src) = NULL;
-#define ISNOTZERO(value) ((value == 0) ? 0 : 1)
-
 /*****************/
 /* MonitoredItem */
 /*****************/
@@ -70,7 +67,7 @@ int MonitoredItem_QueueToDataChangeNotifications(UA_MonitoredItemNotification *d
 
 typedef struct UA_unpublishedNotification {
     LIST_ENTRY(UA_unpublishedNotification) listEntry;
-    UA_NotificationMessage *notification;
+    UA_NotificationMessage notification;
 } UA_unpublishedNotification;
 
 typedef struct UA_Subscription {

+ 16 - 13
src/ua_types.c

@@ -303,7 +303,6 @@ static void Variant_deletemembers(UA_Variant *p, const UA_DataType *dummy) {
 
 static UA_StatusCode
 Variant_copy(UA_Variant const *src, UA_Variant *dst, const UA_DataType *dummy) {
-    UA_Variant_init(dst);
     size_t length = src->arrayLength;
     if(UA_Variant_isScalar(src))
         length = 1;
@@ -585,10 +584,6 @@ DiagnosticInfo_copy(UA_DiagnosticInfo const *src, UA_DiagnosticInfo *dst, const
 /* Structure Types */
 /*******************/
 
-void UA_init(void *p, const UA_DataType *type) {
-    memset(p, 0, type->memSize);
-}
-
 void * UA_new(const UA_DataType *type) {
     void *p = UA_calloc(1, type->memSize);
     return p;
@@ -599,6 +594,8 @@ static UA_StatusCode UA_copyFixedSize(const void *src, void *dst, const UA_DataT
     return UA_STATUSCODE_GOOD;
 }
 
+static UA_StatusCode UA_copyNoInit(const void *src, void *dst, const UA_DataType *type);
+
 typedef UA_StatusCode (*UA_copySignature)(const void *src, void *dst, const UA_DataType *type);
 static const UA_copySignature copyJumpTable[UA_BUILTIN_TYPES_COUNT + 1] = {
     (UA_copySignature)UA_copyFixedSize, // Boolean
@@ -612,24 +609,24 @@ static const UA_copySignature copyJumpTable[UA_BUILTIN_TYPES_COUNT + 1] = {
     (UA_copySignature)UA_copyFixedSize, // UInt64 
     (UA_copySignature)UA_copyFixedSize, // Float 
     (UA_copySignature)UA_copyFixedSize, // Double 
-    (UA_copySignature)UA_copy, // String
+    (UA_copySignature)UA_copyNoInit, // String
     (UA_copySignature)UA_copyFixedSize, // DateTime
     (UA_copySignature)UA_copyFixedSize, // Guid 
-    (UA_copySignature)UA_copy, // ByteString
-    (UA_copySignature)UA_copy, // XmlElement
+    (UA_copySignature)UA_copyNoInit, // ByteString
+    (UA_copySignature)UA_copyNoInit, // XmlElement
     (UA_copySignature)NodeId_copy,
-    (UA_copySignature)UA_copy, // ExpandedNodeId
+    (UA_copySignature)UA_copyNoInit, // ExpandedNodeId
     (UA_copySignature)UA_copyFixedSize, // StatusCode
-    (UA_copySignature)UA_copy, // QualifiedName
-    (UA_copySignature)UA_copy, // LocalizedText
+    (UA_copySignature)UA_copyNoInit, // QualifiedName
+    (UA_copySignature)UA_copyNoInit, // LocalizedText
     (UA_copySignature)ExtensionObject_copy,
     (UA_copySignature)DataValue_copy,
     (UA_copySignature)Variant_copy,
     (UA_copySignature)DiagnosticInfo_copy,
-    (UA_copySignature)UA_copy,
+    (UA_copySignature)UA_copyNoInit,
 };
 
-UA_StatusCode UA_copy(const void *src, void *dst, const UA_DataType *type) {
+static UA_StatusCode UA_copyNoInit(const void *src, void *dst, const UA_DataType *type) {
     UA_StatusCode retval = UA_STATUSCODE_GOOD;
     uintptr_t ptrs = (uintptr_t)src;
     uintptr_t ptrd = (uintptr_t)dst;
@@ -665,8 +662,14 @@ UA_StatusCode UA_copy(const void *src, void *dst, const UA_DataType *type) {
     return retval;
 }
 
+UA_StatusCode UA_copy(const void *src, void *dst, const UA_DataType *type) {
+    memset(dst, 0, type->memSize);
+    return UA_copyNoInit(src, dst, type);
+}
+
 typedef void (*UA_deleteMembersSignature)(void *p, const UA_DataType *type);
 static void nopDeleteMembers(void *p, const UA_DataType *type) { }
+
 static const UA_deleteMembersSignature deleteMembersJumpTable[UA_BUILTIN_TYPES_COUNT + 1] = {
     (UA_deleteMembersSignature)nopDeleteMembers, // Boolean
     (UA_deleteMembersSignature)nopDeleteMembers, // SByte

+ 1 - 0
src/ua_types_encoding_binary.c

@@ -730,6 +730,7 @@ ExtensionObject_decodeBinary(UA_ByteString const *src, size_t *UA_RESTRICT offse
                              UA_ExtensionObject *dst, const UA_DataType *_) {
     UA_Byte encoding = 0;
     UA_NodeId typeId;
+    UA_NodeId_init(&typeId);
     UA_StatusCode retval = NodeId_decodeBinary(src, offset, &typeId, NULL);
     retval |= Byte_decodeBinary(src, offset, &encoding, NULL);
     if(typeId.namespaceIndex != 0 || typeId.identifierType != UA_NODEIDTYPE_NUMERIC)