fix string handling, refactor services

include/ua_types.h

@@ -322,7 +322,7 @@ UA_String UA_EXPORT UA_String_fromChars(char const src[]); ///> Copies the conte
 UA_Boolean UA_EXPORT UA_String_equal(const UA_String *s1, const UA_String *s2); ///> Compares two strings
 UA_StatusCode UA_EXPORT UA_String_copyprintf(char const fmt[], UA_String *dst, ...); ///> Printf a char-array into a UA_String. Memory for the string data is allocated.
 #define UA_STRING_NULL (UA_String){-1, (UA_Byte*)0 }
-#define UA_STRING(CHARS) (UA_String){sizeof(CHARS)-1, (UA_Byte*)CHARS }
+#define UA_STRING(CHARS) (UA_String){strlen(CHARS), (UA_Byte*)CHARS }
 #define UA_STRING_ALLOC(CHARS) UA_String_fromChars(CHARS)
 
 /* DateTime */

src/server/ua_services_attribute.c

@@ -184,7 +184,7 @@ void Service_Read_single(UA_Server *server, UA_Session *session, const UA_Timest
 		return;
 	}
 
-    UA_Node const *node = UA_NodeStore_get(server->nodestore, &(id->nodeId));
+    UA_Node const *node = UA_NodeStore_get(server->nodestore, &id->nodeId);
     if(!node) {
         v->hasStatus = UA_TRUE;
         v->status = UA_STATUSCODE_BADNODEIDUNKNOWN;

src/server/ua_services_nodemanagement.c

@@ -6,22 +6,34 @@
 #include "ua_session.h"
 #include "ua_types_generated_encoding_binary.h"
 
-#define COPY_STANDARDATTRIBUTES do {  \
-    if(attr.specifiedAttributes & UA_NODEATTRIBUTESMASK_DISPLAYNAME) {     \
-        vnode->displayName = attr.displayName;                             \
-        UA_LocalizedText_init(&attr.displayName);                          \
-    }                                                                      \
-    if(attr.specifiedAttributes & UA_NODEATTRIBUTESMASK_DESCRIPTION) {     \
-        vnode->description = attr.description;                             \
-        UA_LocalizedText_init(&attr.description);                          \
-    }                                                                      \
-    if(attr.specifiedAttributes & UA_NODEATTRIBUTESMASK_WRITEMASK)         \
-        vnode->writeMask = attr.writeMask;                                 \
-    if(attr.specifiedAttributes & UA_NODEATTRIBUTESMASK_USERWRITEMASK)     \
-        vnode->userWriteMask = attr.userWriteMask;                         \
-} while(0)
-
-static UA_StatusCode parseVariableNode(UA_ExtensionObject *attributes, UA_Node **new_node) {
+/**
+ * Information Model Consistency
+ *
+ * 
+ *
+ */
+
+/**************************/
+/* Parse Node Definitions */
+/**************************/
+
+/* may _init content in attr */
+static void copyStandardAttributes(UA_Node *node, UA_NodeAttributes *attr) {
+    if(attr->specifiedAttributes & UA_NODEATTRIBUTESMASK_DISPLAYNAME) {
+        node->displayName = attr->displayName;
+        UA_LocalizedText_init(&attr->displayName);
+    }
+    if(attr->specifiedAttributes & UA_NODEATTRIBUTESMASK_DESCRIPTION) {
+        node->description = attr->description;
+        UA_LocalizedText_init(&attr->description);
+    }
+    if(attr->specifiedAttributes & UA_NODEATTRIBUTESMASK_WRITEMASK)
+        node->writeMask = attr->writeMask;
+    if(attr->specifiedAttributes & UA_NODEATTRIBUTESMASK_USERWRITEMASK)
+        node->userWriteMask = attr->userWriteMask;
+}
+
+static UA_StatusCode parseVariableNode(const UA_ExtensionObject *attributes, UA_Node **new_node) {
     if(attributes->typeId.identifier.numeric !=
        UA_TYPES[UA_TYPES_VARIABLEATTRIBUTES].typeId.identifier.numeric + UA_ENCODINGOFFSET_BINARY)
         return UA_STATUSCODE_BADNODEATTRIBUTESINVALID;
@@ -38,8 +50,7 @@ static UA_StatusCode parseVariableNode(UA_ExtensionObject *attributes, UA_Node *
         return UA_STATUSCODE_BADOUTOFMEMORY;
     }
 
-    // now copy all the attributes. This potentially removes them from the decoded attributes.
-    COPY_STANDARDATTRIBUTES;
+    copyStandardAttributes((UA_Node*)vnode, (UA_NodeAttributes*)&attr);
     if(attr.specifiedAttributes & UA_NODEATTRIBUTESMASK_ACCESSLEVEL)
         vnode->accessLevel = attr.accessLevel;
     if(attr.specifiedAttributes & UA_NODEATTRIBUTESMASK_USERACCESSLEVEL)
@@ -77,7 +88,7 @@ static UA_StatusCode parseVariableNode(UA_ExtensionObject *attributes, UA_Node *
     return UA_STATUSCODE_GOOD;
 }
 
-static UA_StatusCode parseObjectNode(UA_ExtensionObject *attributes, UA_Node **new_node) {
+static UA_StatusCode parseObjectNode(const UA_ExtensionObject *attributes, UA_Node **new_node) {
     if(attributes->typeId.identifier.numeric !=
        UA_TYPES[UA_TYPES_OBJECTATTRIBUTES].typeId.identifier.numeric + UA_ENCODINGOFFSET_BINARY)
         return UA_STATUSCODE_BADNODEATTRIBUTESINVALID;
@@ -93,8 +104,7 @@ static UA_StatusCode parseObjectNode(UA_ExtensionObject *attributes, UA_Node **n
         return UA_STATUSCODE_BADOUTOFMEMORY;
     }
 
-    // now copy all the attributes. This potentially removes them from the decoded attributes.
-    COPY_STANDARDATTRIBUTES;
+    copyStandardAttributes((UA_Node*)vnode, (UA_NodeAttributes*)&attr);
     if(attr.specifiedAttributes & UA_NODEATTRIBUTESMASK_EVENTNOTIFIER)
       vnode->eventNotifier = attr.eventNotifier;
     UA_ObjectAttributes_deleteMembers(&attr);
@@ -102,7 +112,7 @@ static UA_StatusCode parseObjectNode(UA_ExtensionObject *attributes, UA_Node **n
     return UA_STATUSCODE_GOOD;
 }
 
-static UA_StatusCode parseReferenceTypeNode(UA_ExtensionObject *attributes, UA_Node **new_node) {
+static UA_StatusCode parseReferenceTypeNode(const UA_ExtensionObject *attributes, UA_Node **new_node) {
     UA_ReferenceTypeAttributes attr;
     size_t pos = 0;
     // todo return more informative error codes from decodeBinary
@@ -114,8 +124,7 @@ static UA_StatusCode parseReferenceTypeNode(UA_ExtensionObject *attributes, UA_N
         return UA_STATUSCODE_BADOUTOFMEMORY;
     }
 
-    // now copy all the attributes. This potentially removes them from the decoded attributes.
-    COPY_STANDARDATTRIBUTES;
+    copyStandardAttributes((UA_Node*)vnode, (UA_NodeAttributes*)&attr);
     if(attr.specifiedAttributes & UA_NODEATTRIBUTESMASK_ISABSTRACT)
         vnode->isAbstract = attr.isAbstract;
     if(attr.specifiedAttributes & UA_NODEATTRIBUTESMASK_SYMMETRIC)
@@ -132,7 +141,7 @@ static UA_StatusCode parseReferenceTypeNode(UA_ExtensionObject *attributes, UA_N
     return UA_STATUSCODE_GOOD;
 }
 
-static UA_StatusCode parseObjectTypeNode(UA_ExtensionObject *attributes, UA_Node **new_node) {
+static UA_StatusCode parseObjectTypeNode(const UA_ExtensionObject *attributes, UA_Node **new_node) {
     UA_ObjectTypeAttributes attr;
     size_t pos = 0;
     // todo return more informative error codes from decodeBinary
@@ -144,17 +153,16 @@ static UA_StatusCode parseObjectTypeNode(UA_ExtensionObject *attributes, UA_Node
         return UA_STATUSCODE_BADOUTOFMEMORY;
     }
     
-    // now copy all the attributes. This potentially removes them from the decoded attributes.
-    COPY_STANDARDATTRIBUTES;
-    if(attr.specifiedAttributes & UA_NODEATTRIBUTESMASK_ISABSTRACT) {
+    copyStandardAttributes((UA_Node*)vnode, (UA_NodeAttributes*)&attr);
+    if(attr.specifiedAttributes & UA_NODEATTRIBUTESMASK_ISABSTRACT)
         vnode->isAbstract = attr.isAbstract;
-    }
+
     UA_ObjectTypeAttributes_deleteMembers(&attr);
     *new_node = (UA_Node*) vnode;
     return UA_STATUSCODE_GOOD;
 }
 
-static UA_StatusCode parseViewNode(UA_ExtensionObject *attributes, UA_Node **new_node) {
+static UA_StatusCode parseViewNode(const UA_ExtensionObject *attributes, UA_Node **new_node) {
     UA_ViewAttributes attr;
     size_t pos = 0;
 
@@ -167,19 +175,23 @@ static UA_StatusCode parseViewNode(UA_ExtensionObject *attributes, UA_Node **new
         return UA_STATUSCODE_BADOUTOFMEMORY;
     }
 
-    // now copy all the attributes. This potentially removes them from the decoded attributes.
-    COPY_STANDARDATTRIBUTES;
+    copyStandardAttributes((UA_Node*)vnode, (UA_NodeAttributes*)&attr);
     if(attr.specifiedAttributes & UA_NODEATTRIBUTESMASK_CONTAINSNOLOOPS)
         vnode->containsNoLoops = attr.containsNoLoops;
     if(attr.specifiedAttributes & UA_NODEATTRIBUTESMASK_EVENTNOTIFIER)
         vnode->eventNotifier = attr.eventNotifier;
+
     UA_ViewAttributes_deleteMembers(&attr);
     *new_node = (UA_Node*) vnode;
     return UA_STATUSCODE_GOOD;
 }
 
-static void addNodeFromAttributes(UA_Server *server, UA_Session *session, UA_AddNodesItem *item,
-                                  UA_AddNodesResult *result) {
+/************/
+/* Add Node */
+/************/
+
+static void Service_AddNodes_single(UA_Server *server, UA_Session *session, const UA_AddNodesItem *item,
+                                    UA_AddNodesResult *result) {
     // adding nodes to ns0 is not allowed over the wire
     if(item->requestedNewNodeId.nodeId.namespaceIndex == 0) {
         result->statusCode = UA_STATUSCODE_BADNODEIDREJECTED;
@@ -237,30 +249,6 @@ static void addNodeFromAttributes(UA_Server *server, UA_Session *session, UA_Add
     }
 }
 
-static UA_StatusCode deleteNode(UA_Server *server, UA_NodeId nodeId, UA_Boolean deleteReferences) {
-  const UA_Node *delNode = UA_NodeStore_get(server->nodestore, &nodeId);
-  if (!delNode)
-    return UA_STATUSCODE_BADNODEIDINVALID;
-  
-  // Find and remove all References to this node if so requested.
-  if(deleteReferences == UA_TRUE) {
-    UA_DeleteReferencesItem *delItem = UA_DeleteReferencesItem_new();
-    delItem->deleteBidirectional = UA_TRUE; // WARNING: Current semantics in deleteOneWayReference is 'delete forward or inverse'
-    UA_NodeId_copy(&nodeId, &delItem->targetNodeId.nodeId);
-    
-    for(int i=0; i<delNode->referencesSize; i++) {
-      UA_NodeId_copy(&delNode->references[i].targetId.nodeId, &delItem->sourceNodeId);
-      
-      UA_NodeId_deleteMembers(&delItem->sourceNodeId);
-    }
-    
-    UA_DeleteReferencesItem_delete(delItem);
-  }
-  
-  UA_NodeStore_release(delNode);
-  return UA_NodeStore_remove(server->nodestore, &nodeId);
-}
-
 void Service_AddNodes(UA_Server *server, UA_Session *session, const UA_AddNodesRequest *request,
                       UA_AddNodesResponse *response) {
     if(request->nodesToAddSize <= 0) {
@@ -307,10 +295,14 @@ void Service_AddNodes(UA_Server *server, UA_Session *session, const UA_AddNodesR
 #ifdef UA_EXTERNAL_NAMESPACES
         if(!isExternal[i])
 #endif
-            addNodeFromAttributes(server, session, &request->nodesToAdd[i], &response->results[i]);
+            Service_AddNodes_single(server, session, &request->nodesToAdd[i], &response->results[i]);
     }
 }
 
+/******************/
+/* Add References */
+/******************/
+
 void Service_AddReferences(UA_Server *server, UA_Session *session, const UA_AddReferencesRequest *request,
                            UA_AddReferencesResponse *response) {
 	if(request->referencesToAddSize <= 0) {
@@ -362,26 +354,57 @@ void Service_AddReferences(UA_Server *server, UA_Session *session, const UA_AddR
 	}
 }
 
+/***************/
+/* Delete Node */
+/***************/
+
+static UA_StatusCode Service_DeleteNodes_single(UA_Server *server, UA_NodeId nodeId,
+                                                UA_Boolean deleteReferences) {
+  const UA_Node *delNode = UA_NodeStore_get(server->nodestore, &nodeId);
+  if (!delNode)
+    return UA_STATUSCODE_BADNODEIDINVALID;
+  
+  // Find and remove all References to this node if so requested.
+  if(deleteReferences == UA_TRUE) {
+    UA_DeleteReferencesItem *delItem = UA_DeleteReferencesItem_new();
+    delItem->deleteBidirectional = UA_TRUE; // WARNING: Current semantics in deleteOneWayReference is 'delete forward or inverse'
+    UA_NodeId_copy(&nodeId, &delItem->targetNodeId.nodeId);
+    
+    for(int i=0; i<delNode->referencesSize; i++) {
+      UA_NodeId_copy(&delNode->references[i].targetId.nodeId, &delItem->sourceNodeId);
+      
+      UA_NodeId_deleteMembers(&delItem->sourceNodeId);
+    }
+    
+    UA_DeleteReferencesItem_delete(delItem);
+  }
+  
+  UA_NodeStore_release(delNode);
+  return UA_NodeStore_remove(server->nodestore, &nodeId);
+}
+
 void Service_DeleteNodes(UA_Server *server, UA_Session *session, const UA_DeleteNodesRequest *request,
                          UA_DeleteNodesResponse *response) {
   UA_StatusCode retval = UA_STATUSCODE_BADSERVICEUNSUPPORTED;
-  
   response->resultsSize = request->nodesToDeleteSize;
   response->results = (UA_StatusCode *) UA_malloc(sizeof(UA_StatusCode) * request->nodesToDeleteSize);
   
   UA_DeleteNodesItem *item;
   for(int i=0; i<request->nodesToDeleteSize; i++) {
     item = &request->nodesToDelete[i];
-    response->results[i] = deleteNode(server, item->nodeId, item->deleteTargetReferences);
+    response->results[i] = Service_DeleteNodes_single(server, item->nodeId, item->deleteTargetReferences);
   }
   
   response->responseHeader.serviceResult = retval;
 }
 
+/********************/
+/* Delete Reference */
+/********************/
+
 void Service_DeleteReferences(UA_Server *server, UA_Session *session,
                               const UA_DeleteReferencesRequest *request,
                               UA_DeleteReferencesResponse *response) {
   UA_StatusCode retval = UA_STATUSCODE_BADSERVICEUNSUPPORTED;
-  
   response->responseHeader.serviceResult = retval;
 }

tests/check_services_attributes.c

@@ -498,7 +498,7 @@ START_TEST(ReadSingleAttributeHistorizingWithoutTimestamp) {
 		Service_Read_single(server, &adminSession, UA_TIMESTAMPSTORETURN_NEITHER, &rReq.nodesToRead[0], &resp);
 		ck_assert_int_eq(-1, resp.value.arrayLength);
 		ck_assert_ptr_eq(&UA_TYPES[UA_TYPES_BOOLEAN], resp.value.type);
-		ck_assert(*(UA_Boolean* )resp.value.data==UA_FALSE);
+		ck_assert(*(UA_Boolean*)resp.value.data==UA_FALSE);
         UA_ReadRequest_deleteMembers(&rReq);
         UA_DataValue_deleteMembers(&resp);
         UA_Server_delete(server);