Fix copy of references. See #1233

open62541 · Nov 9, 2017 · a07c117 · a07c117
1 parent 0f069c5
commit a07c117
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 14 deletions.
diff --git a/include/ua_plugin_nodestore.h b/include/ua_plugin_nodestore.h
@@ -487,6 +487,10 @@ UA_Node_setAttributes(UA_Node *node, const void *attributes,
 UA_StatusCode UA_EXPORT
 UA_Node_copy(const UA_Node *src, UA_Node *dst);
 
+/* Allocate new node and copy the values from src */
+UA_EXPORT UA_Node *
+UA_Node_copy_alloc(const UA_Node *src);
+
 /* Add a single reference to the node */
 UA_StatusCode UA_EXPORT
 UA_Node_addReference(UA_Node *node, const UA_AddReferencesItem *item);

diff --git a/src/server/ua_nodes.c b/src/server/ua_nodes.c
@@ -138,10 +138,9 @@ UA_StatusCode
 UA_Node_copy(const UA_Node *src, UA_Node *dst) {
     if(src->nodeClass != dst->nodeClass)
         return UA_STATUSCODE_BADINTERNALERROR;
-    
+
     /* Copy standard content */
     UA_StatusCode retval = UA_NodeId_copy(&src->nodeId, &dst->nodeId);
-    dst->nodeClass = src->nodeClass;
     retval |= UA_QualifiedName_copy(&src->browseName, &dst->browseName);
     retval |= UA_LocalizedText_copy(&src->displayName, &dst->displayName);
     retval |= UA_LocalizedText_copy(&src->description, &dst->description);
@@ -219,6 +218,48 @@ UA_Node_copy(const UA_Node *src, UA_Node *dst) {
     return retval;
 }
 
+UA_Node *
+UA_Node_copy_alloc(const UA_Node *src) {
+	// use dstPtr to trick static code analysis in accepting dirty cast
+	void *dstPtr;
+	switch(src->nodeClass) {
+		case UA_NODECLASS_OBJECT:
+			dstPtr = UA_malloc(sizeof(UA_ObjectNode));
+			break;
+		case UA_NODECLASS_VARIABLE:
+			dstPtr =UA_malloc(sizeof(UA_VariableNode));
+			break;
+		case UA_NODECLASS_METHOD:
+			dstPtr = UA_malloc(sizeof(UA_MethodNode));
+			break;
+		case UA_NODECLASS_OBJECTTYPE:
+			dstPtr = UA_malloc(sizeof(UA_ObjectTypeNode));
+			break;
+		case UA_NODECLASS_VARIABLETYPE:
+			dstPtr = UA_malloc(sizeof(UA_VariableTypeNode));
+			break;
+		case UA_NODECLASS_REFERENCETYPE:
+			dstPtr = UA_malloc(sizeof(UA_ReferenceTypeNode));
+			break;
+		case UA_NODECLASS_DATATYPE:
+			dstPtr = UA_malloc(sizeof(UA_DataTypeNode));
+			break;
+		case UA_NODECLASS_VIEW:
+			dstPtr = UA_malloc(sizeof(UA_ViewNode));
+			break;
+		default:
+			return NULL;
+	}
+	UA_Node *dst = (UA_Node*)dstPtr;
+	dst->nodeClass = src->nodeClass;
+
+	UA_StatusCode retval = UA_Node_copy(src, dst);
+	if (retval != UA_STATUSCODE_GOOD){
+		UA_free(dst);
+		return NULL;
+	}
+	return dst;
+}
 /******************************/
 /* Copy Attributes into Nodes */
 /******************************/

diff --git a/src/server/ua_server.c b/src/server/ua_server.c
@@ -57,24 +57,27 @@ UA_Server_forEachChildNodeCall(UA_Server *server, UA_NodeId parentNodeId,
     /* TODO: We need to do an ugly copy of the references array since users may
      * delete references from within the callback. In single-threaded mode this
      * changes the same node we point at here. In multi-threaded mode, this
-     * creates a new copy as nodes are truly immutable. */
-    UA_ReferenceNode *refs = NULL;
-    size_t refssize = parent->referencesSize;
-    UA_StatusCode retval = UA_Array_copy(parent->references, parent->referencesSize,
-        (void**)&refs, &UA_TYPES[UA_TYPES_REFERENCENODE]);
-    if(retval != UA_STATUSCODE_GOOD) {
+     * creates a new copy as nodes are truly immutable.
+     * The callback could remove a node via the regular public API.
+     * This can remove a member of the nodes-array we iterate over...
+     * */
+    UA_Node *parentCopy = UA_Node_copy_alloc(parent);
+    if(!parentCopy) {
         server->config.nodestore.releaseNode(server->config.nodestore.context, parent);
-        return retval;
+        return UA_STATUSCODE_BADUNEXPECTEDERROR;
     }
 
-    for(size_t i = parent->referencesSize; i > 0; --i) {
-        UA_ReferenceNode *ref = &refs[i - 1];
-        retval |= callback(ref->targetId.nodeId, ref->isInverse,
-                           ref->referenceTypeId, handle);
+    UA_StatusCode retval = UA_STATUSCODE_GOOD;
+    for(size_t i = parentCopy->referencesSize; i > 0; --i) {
+        UA_NodeReferenceKind *ref = &parentCopy->references[i - 1];
+        for (size_t j = 0; j<ref->targetIdsSize; j++)
+            retval |= callback(ref->targetIds[j].nodeId, ref->isInverse,
+                               ref->referenceTypeId, handle);
     }
+    UA_Node_deleteMembers(parentCopy);
+    UA_free(parentCopy);
 
     server->config.nodestore.releaseNode(server->config.nodestore.context, parent);
-    UA_Array_delete(refs, refssize, &UA_TYPES[UA_TYPES_REFERENCENODE]);
     return retval;
 }