Server: Don't return shallow copies from Read service

src/server/ua_services_attribute.c

@@ -75,21 +75,6 @@ getUserExecutable(UA_Server *server, const UA_Session *session,
 /* Read Service */
 /****************/
 
-static UA_StatusCode
-readArrayDimensionsAttribute(const UA_VariableNode *vn, UA_DataValue *v) {
-    UA_Variant_setArray(&v->value, vn->arrayDimensions,
-                        vn->arrayDimensionsSize, &UA_TYPES[UA_TYPES_UINT32]);
-    v->value.storageType = UA_VARIANT_DATA_NODELETE;
-    return UA_STATUSCODE_GOOD;
-}
-
-static void
-setScalarNoDelete(UA_Variant *v, const void * UA_RESTRICT p,
-                  const UA_DataType *type) {
-    UA_Variant_setScalar(v, (void*)(uintptr_t)p, type);
-    v->storageType = UA_VARIANT_DATA_NODELETE;
-}
-
 static UA_StatusCode
 readIsAbstractAttribute(const UA_Node *node, UA_Variant *v) {
     const UA_Boolean *isAbstract;
@@ -147,8 +132,18 @@ readValueAttributeFromDataSource(UA_Server *server, UA_Session *session,
         return UA_STATUSCODE_BADINTERNALERROR;
     UA_Boolean sourceTimeStamp = (timestamps == UA_TIMESTAMPSTORETURN_SOURCE ||
                                   timestamps == UA_TIMESTAMPSTORETURN_BOTH);
-    return vn->value.dataSource.read(server, &session->sessionId, session->sessionHandle,
-                                     &vn->nodeId, vn->context, sourceTimeStamp, rangeptr, v);
+    UA_DataValue v2;
+    UA_DataValue_init(&v2);
+    UA_StatusCode retval = vn->value.dataSource.
+        read(server, &session->sessionId, session->sessionHandle,
+             &vn->nodeId, vn->context, sourceTimeStamp, rangeptr, &v2);
+    if(v2.hasValue && v2.value.storageType == UA_VARIANT_DATA_NODELETE) {
+        retval = UA_DataValue_copy(&v2, v);
+        UA_DataValue_deleteMembers(&v2);
+    } else {
+        *v = v2;
+    }
+    return retval;
 }
 
 static UA_StatusCode
@@ -227,22 +222,22 @@ ReadWithNode(const UA_Node *node, UA_Server *server, UA_Session *session,
     UA_StatusCode retval = UA_STATUSCODE_GOOD;
     switch(id->attributeId) {
     case UA_ATTRIBUTEID_NODEID:
-        setScalarNoDelete(&v->value, &node->nodeId, &UA_TYPES[UA_TYPES_NODEID]);
+        retval = UA_Variant_setScalarCopy(&v->value, &node->nodeId, &UA_TYPES[UA_TYPES_NODEID]);
         break;
     case UA_ATTRIBUTEID_NODECLASS:
-        setScalarNoDelete(&v->value, &node->nodeClass, &UA_TYPES[UA_TYPES_NODECLASS]);
+        retval = UA_Variant_setScalarCopy(&v->value, &node->nodeClass, &UA_TYPES[UA_TYPES_NODECLASS]);
         break;
     case UA_ATTRIBUTEID_BROWSENAME:
-        setScalarNoDelete(&v->value, &node->browseName, &UA_TYPES[UA_TYPES_QUALIFIEDNAME]);
+        retval = UA_Variant_setScalarCopy(&v->value, &node->browseName, &UA_TYPES[UA_TYPES_QUALIFIEDNAME]);
         break;
     case UA_ATTRIBUTEID_DISPLAYNAME:
-        setScalarNoDelete(&v->value, &node->displayName, &UA_TYPES[UA_TYPES_LOCALIZEDTEXT]);
+        retval = UA_Variant_setScalarCopy(&v->value, &node->displayName, &UA_TYPES[UA_TYPES_LOCALIZEDTEXT]);
         break;
     case UA_ATTRIBUTEID_DESCRIPTION:
-        setScalarNoDelete(&v->value, &node->description, &UA_TYPES[UA_TYPES_LOCALIZEDTEXT]);
+        retval = UA_Variant_setScalarCopy(&v->value, &node->description, &UA_TYPES[UA_TYPES_LOCALIZEDTEXT]);
         break;
     case UA_ATTRIBUTEID_WRITEMASK:
-        setScalarNoDelete(&v->value, &node->writeMask, &UA_TYPES[UA_TYPES_UINT32]);
+        retval = UA_Variant_setScalarCopy(&v->value, &node->writeMask, &UA_TYPES[UA_TYPES_UINT32]);
         break;
     case UA_ATTRIBUTEID_USERWRITEMASK: {
         UA_UInt32 userWriteMask = getUserWriteMask(server, session, node);
@@ -253,27 +248,27 @@ ReadWithNode(const UA_Node *node, UA_Server *server, UA_Session *session,
         break;
     case UA_ATTRIBUTEID_SYMMETRIC:
         CHECK_NODECLASS(UA_NODECLASS_REFERENCETYPE);
-        setScalarNoDelete(&v->value, &((const UA_ReferenceTypeNode*)node)->symmetric,
+        retval = UA_Variant_setScalarCopy(&v->value, &((const UA_ReferenceTypeNode*)node)->symmetric,
                           &UA_TYPES[UA_TYPES_BOOLEAN]);
         break;
     case UA_ATTRIBUTEID_INVERSENAME:
         CHECK_NODECLASS(UA_NODECLASS_REFERENCETYPE);
-        setScalarNoDelete(&v->value, &((const UA_ReferenceTypeNode*)node)->inverseName,
+        retval = UA_Variant_setScalarCopy(&v->value, &((const UA_ReferenceTypeNode*)node)->inverseName,
                           &UA_TYPES[UA_TYPES_LOCALIZEDTEXT]);
         break;
     case UA_ATTRIBUTEID_CONTAINSNOLOOPS:
         CHECK_NODECLASS(UA_NODECLASS_VIEW);
-        setScalarNoDelete(&v->value, &((const UA_ViewNode*)node)->containsNoLoops,
+        retval = UA_Variant_setScalarCopy(&v->value, &((const UA_ViewNode*)node)->containsNoLoops,
                           &UA_TYPES[UA_TYPES_BOOLEAN]);
         break;
     case UA_ATTRIBUTEID_EVENTNOTIFIER:
         CHECK_NODECLASS(UA_NODECLASS_VIEW | UA_NODECLASS_OBJECT);
         if(node->nodeClass == UA_NODECLASS_VIEW) {
-            setScalarNoDelete(&v->value, &((const UA_ViewNode*)node)->eventNotifier,
+            retval = UA_Variant_setScalarCopy(&v->value, &((const UA_ViewNode*)node)->eventNotifier,
                               &UA_TYPES[UA_TYPES_BYTE]);
         }
         else{
-            setScalarNoDelete(&v->value, &((const UA_ObjectNode*)node)->eventNotifier,
+            retval = UA_Variant_setScalarCopy(&v->value, &((const UA_ObjectNode*)node)->eventNotifier,
                               &UA_TYPES[UA_TYPES_BYTE]);
         }
         break;
@@ -301,48 +296,48 @@ ReadWithNode(const UA_Node *node, UA_Server *server, UA_Session *session,
     }
     case UA_ATTRIBUTEID_DATATYPE:
         CHECK_NODECLASS(UA_NODECLASS_VARIABLE | UA_NODECLASS_VARIABLETYPE);
-        setScalarNoDelete(&v->value, &((const UA_VariableTypeNode*)node)->dataType,
-                          &UA_TYPES[UA_TYPES_NODEID]);
+        retval = UA_Variant_setScalarCopy(&v->value, &((const UA_VariableTypeNode*)node)->dataType,
+                                          &UA_TYPES[UA_TYPES_NODEID]);
         break;
     case UA_ATTRIBUTEID_VALUERANK:
         CHECK_NODECLASS(UA_NODECLASS_VARIABLE | UA_NODECLASS_VARIABLETYPE);
-        setScalarNoDelete(&v->value, &((const UA_VariableTypeNode*)node)->valueRank,
-                          &UA_TYPES[UA_TYPES_INT32]);
+        retval = UA_Variant_setScalarCopy(&v->value, &((const UA_VariableTypeNode*)node)->valueRank,
+                                          &UA_TYPES[UA_TYPES_INT32]);
         break;
     case UA_ATTRIBUTEID_ARRAYDIMENSIONS:
         CHECK_NODECLASS(UA_NODECLASS_VARIABLE | UA_NODECLASS_VARIABLETYPE);
-        retval = readArrayDimensionsAttribute((const UA_VariableNode*)node, v);
+        retval = UA_Variant_setArrayCopy(&v->value, ((const UA_VariableTypeNode*)node)->arrayDimensions,
+                                         ((const UA_VariableTypeNode*)node)->arrayDimensionsSize,
+                                         &UA_TYPES[UA_TYPES_UINT32]);
         break;
     case UA_ATTRIBUTEID_ACCESSLEVEL:
         CHECK_NODECLASS(UA_NODECLASS_VARIABLE);
-        setScalarNoDelete(&v->value, &((const UA_VariableNode*)node)->accessLevel,
-                          &UA_TYPES[UA_TYPES_BYTE]);
+        retval = UA_Variant_setScalarCopy(&v->value, &((const UA_VariableNode*)node)->accessLevel,
+                                          &UA_TYPES[UA_TYPES_BYTE]);
         break;
     case UA_ATTRIBUTEID_USERACCESSLEVEL: {
         CHECK_NODECLASS(UA_NODECLASS_VARIABLE);
-        UA_Byte userAccessLevel = getUserAccessLevel(server, session,
-                                                     (const UA_VariableNode*)node);
+        UA_Byte userAccessLevel = getUserAccessLevel(server, session, (const UA_VariableNode*)node);
         retval = UA_Variant_setScalarCopy(&v->value, &userAccessLevel, &UA_TYPES[UA_TYPES_BYTE]);
         break; }
     case UA_ATTRIBUTEID_MINIMUMSAMPLINGINTERVAL:
         CHECK_NODECLASS(UA_NODECLASS_VARIABLE);
-        setScalarNoDelete(&v->value, &((const UA_VariableNode*)node)->minimumSamplingInterval,
+        retval = UA_Variant_setScalarCopy(&v->value, &((const UA_VariableNode*)node)->minimumSamplingInterval,
                           &UA_TYPES[UA_TYPES_DOUBLE]);
         break;
     case UA_ATTRIBUTEID_HISTORIZING:
         CHECK_NODECLASS(UA_NODECLASS_VARIABLE);
-        setScalarNoDelete(&v->value, &((const UA_VariableNode*)node)->historizing,
+        retval = UA_Variant_setScalarCopy(&v->value, &((const UA_VariableNode*)node)->historizing,
                           &UA_TYPES[UA_TYPES_BOOLEAN]);
         break;
     case UA_ATTRIBUTEID_EXECUTABLE:
         CHECK_NODECLASS(UA_NODECLASS_METHOD);
-        setScalarNoDelete(&v->value, &((const UA_MethodNode*)node)->executable,
+        retval = UA_Variant_setScalarCopy(&v->value, &((const UA_MethodNode*)node)->executable,
                           &UA_TYPES[UA_TYPES_BOOLEAN]);
         break;
     case UA_ATTRIBUTEID_USEREXECUTABLE: {
         CHECK_NODECLASS(UA_NODECLASS_METHOD);
-        UA_Boolean userExecutable = getUserExecutable(server, session,
-                                                      (const UA_MethodNode*)node);
+        UA_Boolean userExecutable = getUserExecutable(server, session, (const UA_MethodNode*)node);
         retval = UA_Variant_setScalarCopy(&v->value, &userExecutable, &UA_TYPES[UA_TYPES_BOOLEAN]);
         break; }
     default:
@@ -449,19 +444,6 @@ UA_Server_readWithSession(UA_Server *server, UA_Session *session,
     /* Perform the read operation */
     ReadWithNode(node, server, session, timestampsToReturn, item, &dv);
 
-    /* Do we have to copy the result before releasing the node? */
-    if(dv.hasValue && dv.value.storageType == UA_VARIANT_DATA_NODELETE) {
-        UA_DataValue dv2;
-        UA_StatusCode retval = UA_DataValue_copy(&dv, &dv2);
-        if(retval == UA_STATUSCODE_GOOD) {
-            dv = dv2;
-        } else {
-            UA_DataValue_init(&dv);
-            dv.hasStatus = true;
-            dv.status = retval;
-        }
-    }
-
     /* Release the node and return */
     UA_Nodestore_releaseNode(server->nsCtx, node);
     return dv;

src/server/ua_services_nodemanagement.c

@@ -190,6 +190,7 @@ typeCheckVariableNode(UA_Server *server, UA_Session *session,
                               "AddNodes: The value of %.*s is incompatible with "
                               "the datatype of the VariableType",
                               (int)nodeIdStr.length, nodeIdStr.data));
+        UA_DataValue_deleteMembers(&value);
         return UA_STATUSCODE_BADTYPEMISMATCH;
     }
 
@@ -199,6 +200,7 @@ typeCheckVariableNode(UA_Server *server, UA_Session *session,
         UA_LOG_NODEID_WRAP(&node->nodeId, UA_LOG_INFO_SESSION(&server->config.logger, session,
                            "AddNodes: The value rank of %.*s is incomatible "
                            "with its array dimensions", (int)nodeIdStr.length, nodeIdStr.data));
+        UA_DataValue_deleteMembers(&value);
         return UA_STATUSCODE_BADTYPEMISMATCH;
     }
 
@@ -208,6 +210,7 @@ typeCheckVariableNode(UA_Server *server, UA_Session *session,
                            "AddNodes: The value rank of %.*s is incomatible "
                            "with the value rank of the VariableType",
                            (int)nodeIdStr.length, nodeIdStr.data));
+        UA_DataValue_deleteMembers(&value);
         return UA_STATUSCODE_BADTYPEMISMATCH;
     }
 
@@ -218,6 +221,7 @@ typeCheckVariableNode(UA_Server *server, UA_Session *session,
                            "AddNodes: The array dimensions of %.*s are "
                            "incomatible with the array dimensions of the VariableType",
                            (int)nodeIdStr.length, nodeIdStr.data));
+        UA_DataValue_deleteMembers(&value);
         return UA_STATUSCODE_BADTYPEMISMATCH;
     }