Revert "refactor: decompose large attribute read/write functions"

This reverts commit 23a6e70ce5832aac526bbe3b13c7454bb97c892f.

src/server/ua_services_attribute.c

@@ -103,43 +103,6 @@ compatibleArrayDimensions(size_t constraintArrayDimensionsSize,
     return UA_STATUSCODE_GOOD;
 }
 
-/* Returns the pointer to a datavalue with a possibly transformed type to match
-   the description */
-static const UA_Variant *
-convertToMatchingValue(UA_Server *server, const UA_Variant *value,
-                       const UA_NodeId *targetDataTypeId, UA_Variant *editableValue) {
-    const UA_DataType *targetDataType = findDataType(targetDataTypeId);
-    if(!targetDataType)
-        return NULL;
-
-    /* A string is written to a byte array. the valuerank and array
-       dimensions are checked later */
-    if(targetDataType == &UA_TYPES[UA_TYPES_BYTE] &&
-       value->type == &UA_TYPES[UA_TYPES_BYTESTRING] &&
-       UA_Variant_isScalar(value)) {
-        UA_ByteString *str = (UA_ByteString*)value->data;
-        editableValue->storageType = UA_VARIANT_DATA_NODELETE;
-        editableValue->type = &UA_TYPES[UA_TYPES_BYTE];
-        editableValue->arrayLength = str->length;
-        editableValue->data = str->data;
-        return editableValue;
-    }
-
-    /* An enum was sent as an int32, or an opaque type as a bytestring. This
-     * is detected with the typeIndex indicating the "true" datatype. */
-    enum type_equivalence te1 = typeEquivalence(targetDataType);
-    enum type_equivalence te2 = typeEquivalence(value->type);
-    if(te1 != TYPE_EQUIVALENCE_NONE && te1 == te2) {
-        *editableValue = *value;
-        editableValue->storageType = UA_VARIANT_DATA_NODELETE;
-        editableValue->type = targetDataType;
-        return editableValue;
-    }
-
-    /* No more possible equivalencies */
-    return NULL;
-}
-
 /* Test whether the value matches a variable definition given by
  * - datatype
  * - valueranke
@@ -148,51 +111,77 @@ convertToMatchingValue(UA_Server *server, const UA_Variant *value,
  * byte array to bytestring or uint32 to some enum. If editableValue is non-NULL,
  * we try to create a matching variant that points to the original data. */
 UA_StatusCode
-typeCheckValue(UA_Server *server, const UA_NodeId *targetDataTypeId,
-               UA_Int32 targetValueRank, size_t targetArrayDimensionsSize,
-               const UA_UInt32 *targetArrayDimensions, const UA_Variant *value,
+typeCheckValue(UA_Server *server, const UA_NodeId *variableDataTypeId,
+               UA_Int32 variableValueRank, size_t variableArrayDimensionsSize,
+               const UA_UInt32 *variableArrayDimensions, const UA_Variant *value,
                const UA_NumericRange *range, UA_Variant *editableValue) {
-    /* Empty variant is only allowed for BaseDataType */
+    /* No content is only allowed for BaseDataType */
+    const UA_NodeId *valueDataTypeId;
     UA_NodeId basedatatype = UA_NODEID_NUMERIC(0, UA_NS0ID_BASEDATATYPE);
-    if(!value->type) {
-        if(UA_NodeId_equal(targetDataTypeId, &basedatatype))
-            goto check_array;
-        else
-            return UA_STATUSCODE_BADTYPEMISMATCH;
-    }
+    if(value->type)
+        valueDataTypeId = &value->type->typeId;
+    else
+        valueDataTypeId = &basedatatype;
     
     /* See if the types match. The nodeid on the wire may be != the nodeid in
      * the node for opaque types, enums and bytestrings. value contains the
      * correct type definition after the following paragraph */
-    if(UA_NodeId_equal(&value->type->typeId, targetDataTypeId))
-        goto check_array;
+    if(!UA_NodeId_equal(valueDataTypeId, variableDataTypeId)) {
+        /* contains the value a subtype of the required type? */
+        const UA_NodeId subtypeId = UA_NODEID_NUMERIC(0, UA_NS0ID_HASSUBTYPE);
+        if(isNodeInTree(server->nodestore, valueDataTypeId,
+                        variableDataTypeId, &subtypeId, 1))
+            goto check_array;
 
-    /* Has the value a subtype of the required type? */
-    const UA_NodeId subtypeId = UA_NODEID_NUMERIC(0, UA_NS0ID_HASSUBTYPE);
-    if(isNodeInTree(server->nodestore, &value->type->typeId, targetDataTypeId, &subtypeId, 1))
-        goto check_array;
+        const UA_DataType *variableDataType = findDataType(variableDataTypeId);
+        const UA_DataType *valueDataType = findDataType(valueDataTypeId);
+        if(!editableValue || !variableDataType || !valueDataType)
+            return UA_STATUSCODE_BADTYPEMISMATCH;
 
-    /* Try to convert to a matching value if this is wanted */
-    if(!editableValue)
-        return UA_STATUSCODE_BADTYPEMISMATCH;
-    value = convertToMatchingValue(server, value, targetDataTypeId, editableValue);
-    if(!value)
+        /* a string is written to a byte array. the valuerank and array
+           dimensions are checked later */
+        if(variableDataType == &UA_TYPES[UA_TYPES_BYTE] &&
+           valueDataType == &UA_TYPES[UA_TYPES_BYTESTRING] &&
+           !range && UA_Variant_isScalar(value)) {
+            UA_ByteString *str = (UA_ByteString*)value->data;
+            editableValue->storageType = UA_VARIANT_DATA_NODELETE;
+            editableValue->type = &UA_TYPES[UA_TYPES_BYTE];
+            editableValue->arrayLength = str->length;
+            editableValue->data = str->data;
+            value = editableValue;
+            goto check_array;
+        }
+
+        /* An enum was sent as an int32, or an opaque type as a bytestring. This
+         * is detected with the typeIndex indicating the "true" datatype. */
+        enum type_equivalence te1 = typeEquivalence(variableDataType);
+        enum type_equivalence te2 = typeEquivalence(valueDataType);
+        if(te1 != TYPE_EQUIVALENCE_NONE && te1 == te2) {
+            *editableValue = *value;
+            editableValue->storageType = UA_VARIANT_DATA_NODELETE;
+            editableValue->type = variableDataType;
+            value = editableValue;
+            goto check_array;
+        }
+
+        /* No more possible equivalencies */
         return UA_STATUSCODE_BADTYPEMISMATCH;
+    }
 
  check_array:
-    if(range) /* array dimensions are checked later when writing the range */
+    if(range) /* array dimensions are checked when writing the range */
         return UA_STATUSCODE_GOOD;
 
     /* See if the array dimensions match. When arrayDimensions are defined, they
      * already hold the valuerank. */
-    if(targetArrayDimensionsSize > 0)
-        return compatibleArrayDimensions(targetArrayDimensionsSize,
-                                         targetArrayDimensions,
+    if(variableArrayDimensionsSize > 0)
+        return compatibleArrayDimensions(variableArrayDimensionsSize,
+                                         variableArrayDimensions,
                                          value->arrayDimensionsSize,
                                          value->arrayDimensions);
 
     /* Check if the valuerank allows for the value dimension */
-    return compatibleValueRankValue(targetValueRank, value);
+    return compatibleValueRankValue(variableValueRank, value);
 }
 
 /*****************************/
@@ -405,51 +394,43 @@ writeDataTypeAttribute(UA_Server *server, UA_VariableNode *node,
 /* Value Attribute */
 /*******************/
 
-static UA_StatusCode
-readValueAttributeFromNode(const UA_VariableNode *vn, UA_DataValue *v,
-                           UA_NumericRange *rangeptr) {
-    if(vn->value.data.callback.onRead)
-        vn->value.data.callback.onRead(vn->value.data.callback.handle,
-                                       vn->nodeId, &v->value, rangeptr);
-    if(rangeptr)
-        return UA_Variant_copyRange(&vn->value.data.value.value, &v->value, *rangeptr);
-    *v = vn->value.data.value;
-    v->value.storageType = UA_VARIANT_DATA_NODELETE;
-    return UA_STATUSCODE_GOOD;
-}
-
-static UA_StatusCode
-readValueAttributeFromDataSource(const UA_VariableNode *vn, UA_DataValue *v,
-                                 UA_TimestampsToReturn timestamps,
-                                 UA_NumericRange *rangeptr) {
-    if(!vn->value.dataSource.read)
-        return UA_STATUSCODE_BADINTERNALERROR;
-    UA_Boolean sourceTimeStamp = (timestamps == UA_TIMESTAMPSTORETURN_SOURCE ||
-                                  timestamps == UA_TIMESTAMPSTORETURN_BOTH);
-    return vn->value.dataSource.read(vn->value.dataSource.handle, vn->nodeId,
-                                     sourceTimeStamp, rangeptr, v);
-}
-
 static UA_StatusCode
 readValueAttributeComplete(const UA_VariableNode *vn, UA_TimestampsToReturn timestamps,
-                           const UA_String *indexRange, UA_DataValue *v) {
+                           const UA_ReadValueId *id, UA_DataValue *v) {
     /* Compute the index range */
     UA_NumericRange range;
     UA_NumericRange *rangeptr = NULL;
     UA_StatusCode retval = UA_STATUSCODE_GOOD;
-    if(indexRange) {
-        retval = parse_numericrange(indexRange, &range);
+    if(id->indexRange.length > 0) {
+        retval = parse_numericrange(&id->indexRange, &range);
         if(retval != UA_STATUSCODE_GOOD)
             return retval;
         rangeptr = ⦥
     }
 
-    /* Read the value */
-    if(vn->valueSource == UA_VALUESOURCE_DATA)
-        retval = readValueAttributeFromNode(vn, v, rangeptr);
-    else 
-        retval = readValueAttributeFromDataSource(vn, v, timestamps, rangeptr);
-    
+    if(vn->valueSource == UA_VALUESOURCE_DATA) {
+        /* Data in the node */
+        if(vn->value.data.callback.onRead)
+            vn->value.data.callback.onRead(vn->value.data.callback.handle,
+                                           vn->nodeId, &v->value, rangeptr);
+        if(!rangeptr) {
+            *v = vn->value.data.value;
+            v->value.storageType = UA_VARIANT_DATA_NODELETE;
+        } else {
+            retval = UA_Variant_copyRange(&vn->value.data.value.value, &v->value, range);
+        }
+    } else {
+        /* Data in a value source */
+        if(!vn->value.dataSource.read) {
+            retval = UA_STATUSCODE_BADINTERNALERROR;
+        } else {
+            UA_Boolean sourceTimeStamp = (timestamps == UA_TIMESTAMPSTORETURN_SOURCE ||
+                                          timestamps == UA_TIMESTAMPSTORETURN_BOTH);
+            retval = vn->value.dataSource.read(vn->value.dataSource.handle, vn->nodeId,
+                                               sourceTimeStamp, rangeptr, v);
+        }
+    }
+
     /* Clean up */
     if(rangeptr)
         UA_free(range.dimensions);
@@ -458,51 +439,55 @@ readValueAttributeComplete(const UA_VariableNode *vn, UA_TimestampsToReturn time
 
 UA_StatusCode
 readValueAttribute(const UA_VariableNode *vn, UA_DataValue *v) {
-    return readValueAttributeComplete(vn, UA_TIMESTAMPSTORETURN_NEITHER, NULL, v);
-}
-
-static UA_StatusCode
-writeValueAttributeWithoutRange(UA_VariableNode *node, const UA_DataValue *value) {
-    UA_DataValue old_value = node->value.data.value; /* keep the pointers for restoring */
-    UA_StatusCode retval = UA_DataValue_copy(value, &node->value.data.value);
-    if(retval == UA_STATUSCODE_GOOD)
-        UA_DataValue_deleteMembers(&old_value);
-    else
-        node->value.data.value = old_value; 
-    return retval;
+    UA_TimestampsToReturn timestamps = UA_TIMESTAMPSTORETURN_NEITHER;
+    UA_ReadValueId id;
+    UA_ReadValueId_init(&id);
+    return readValueAttributeComplete(vn, timestamps, &id, v);
 }
 
 static UA_StatusCode
-writeValueAttributeWithRange(UA_VariableNode *node, const UA_DataValue *value,
-                             const UA_NumericRange *rangeptr) {
-    /* Value on both sides? */
-    if(value->status != node->value.data.value.status ||
-       !value->hasValue || !node->value.data.value.hasValue)
-        return UA_STATUSCODE_BADINDEXRANGEINVALID;
-
-    /* Make scalar a one-entry array for range matching */
-    UA_Variant editableValue;
-    const UA_Variant *v = &value->value;
-    if(UA_Variant_isScalar(&value->value)) {
-        editableValue = value->value;
-        editableValue.arrayLength = 1;
-        v = &editableValue;
-    }
+writeValueAttributeAfterTypeCheck(UA_VariableNode *node, const UA_DataValue *value,
+                                  const UA_NumericRange *rangeptr) {
+    UA_StatusCode retval = UA_STATUSCODE_GOOD;
+    if(!rangeptr) {
+        /* Replace without a range */
+        UA_DataValue old_value = node->value.data.value; /* keep the pointers for restoring */
+        retval = UA_DataValue_copy(value, &node->value.data.value);
+        if(retval == UA_STATUSCODE_GOOD)
+            UA_DataValue_deleteMembers(&old_value);
+        else
+            node->value.data.value = old_value; 
+    } else {
+        /* Write into a range */
+        if(value->status == UA_STATUSCODE_GOOD) {
+            if(!node->value.data.value.hasValue || !value->hasValue)
+                return UA_STATUSCODE_BADINDEXRANGEINVALID;
+
+            /* Make scalar a one-entry array for range matching */
+            UA_Variant editableValue;
+            const UA_Variant *v = &value->value;
+            if(UA_Variant_isScalar(&value->value)) {
+                editableValue = value->value;
+                editableValue.arrayLength = 1;
+                v = &editableValue;
+            }
                 
-    /* Write the value */
-    UA_StatusCode retval = UA_Variant_setRangeCopy(&node->value.data.value.value,
-                                                   v->data, v->arrayLength, *rangeptr);
-    if(retval != UA_STATUSCODE_GOOD)
-        return retval;
+            /* Write the value */
+            retval = UA_Variant_setRangeCopy(&node->value.data.value.value,
+                                             v->data, v->arrayLength, *rangeptr);
+            if(retval != UA_STATUSCODE_GOOD)
+                return retval;
+        }
 
-    /* Write the status and timestamps */
-    node->value.data.value.hasStatus = value->hasStatus;
-    node->value.data.value.status = value->status;
-    node->value.data.value.hasSourceTimestamp = value->hasSourceTimestamp;
-    node->value.data.value.sourceTimestamp = value->sourceTimestamp;
-    node->value.data.value.hasSourcePicoseconds = value->hasSourcePicoseconds;
-    node->value.data.value.sourcePicoseconds = value->sourcePicoseconds;
-    return UA_STATUSCODE_GOOD;
+        /* Write the status and timestamps */
+        node->value.data.value.hasStatus = value->hasStatus;
+        node->value.data.value.status = value->status;
+        node->value.data.value.hasSourceTimestamp = value->hasSourceTimestamp;
+        node->value.data.value.sourceTimestamp = value->sourceTimestamp;
+        node->value.data.value.hasSourcePicoseconds = value->hasSourcePicoseconds;
+        node->value.data.value.sourcePicoseconds = value->sourcePicoseconds;
+    }
+    return retval;
 }
 
 UA_StatusCode
@@ -524,28 +509,27 @@ writeValueAttribute(UA_Server *server, UA_VariableNode *node,
     UA_DataValue editableValue = *value;
     editableValue.value.storageType = UA_VARIANT_DATA_NODELETE;
 
+    /* Set the source timestamp if there is none */
+    if(!editableValue.hasSourceTimestamp) {
+        editableValue.sourceTimestamp = UA_DateTime_now();
+        editableValue.hasSourceTimestamp = true;
+    }
+
     /* Type checking. May change the type of editableValue */
     if(value->hasValue) {
         retval = typeCheckValue(server, &node->dataType, node->valueRank,
                                 node->arrayDimensionsSize, node->arrayDimensions,
                                 &value->value, rangeptr, &editableValue.value);
-        if(retval != UA_STATUSCODE_GOOD)
+        if(retval != UA_STATUSCODE_GOOD) {
+            UA_LOG_DEBUG(server->config.logger, UA_LOGCATEGORY_SERVER,
+                         "The new value does not match the variable definiton");
             goto cleanup;
-    }
-
-    /* Set the source timestamp if there is none */
-    if(!editableValue.hasSourceTimestamp) {
-        editableValue.sourceTimestamp = UA_DateTime_now();
-        editableValue.hasSourceTimestamp = true;
+        }
     }
 
     /* Ok, do it */
     if(node->valueSource == UA_VALUESOURCE_DATA) {
-        if(!rangeptr)
-            retval = writeValueAttributeWithoutRange(node, &editableValue);
-        else
-            retval = writeValueAttributeWithRange(node, &editableValue, rangeptr);
-        /* Callback after writing */
+        retval = writeValueAttributeAfterTypeCheck(node, &editableValue, rangeptr);
         if(retval == UA_STATUSCODE_GOOD && node->value.data.callback.onWrite)
             node->value.data.callback.onWrite(node->value.data.callback.handle, node->nodeId,
                                               &node->value.data.value.value, rangeptr);
@@ -617,6 +601,7 @@ writeIsAbstractAttribute(UA_Node *node, UA_Boolean value) {
 /****************/
 
 static const UA_String binEncoding = {sizeof("DefaultBinary")-1, (UA_Byte*)"DefaultBinary"};
+/* clang complains about unused variables */
 /* static const UA_String xmlEncoding = {sizeof("DefaultXml")-1, (UA_Byte*)"DefaultXml"}; */
 
 #define CHECK_NODECLASS(CLASS)                                  \
@@ -639,8 +624,8 @@ void Service_Read_single(UA_Server *server, UA_Session *session,
            return;
     }
 
-    /* Index range for an attribute other than value */
-    if(id->indexRange.length > 0 && id->attributeId != UA_ATTRIBUTEID_VALUE) {
+    /* index range for a non-value */
+    if(id->indexRange.length > 0 && id->attributeId != UA_ATTRIBUTEID_VALUE){
         v->hasStatus = true;
         v->status = UA_STATUSCODE_BADINDEXRANGENODATA;
         return;
@@ -654,7 +639,7 @@ void Service_Read_single(UA_Server *server, UA_Session *session,
         return;
     }
 
-    /* Read the attribute */
+    /* Read the value */
     UA_StatusCode retval = UA_STATUSCODE_GOOD;
     switch(id->attributeId) {
     case UA_ATTRIBUTEID_NODEID:
@@ -704,7 +689,7 @@ void Service_Read_single(UA_Server *server, UA_Session *session,
     case UA_ATTRIBUTEID_VALUE:
         CHECK_NODECLASS(UA_NODECLASS_VARIABLE | UA_NODECLASS_VARIABLETYPE);
         retval = readValueAttributeComplete((const UA_VariableNode*)node,
-                                            timestamps, &id->indexRange, v);
+                                            timestamps, id, v);
         break;
     case UA_ATTRIBUTEID_DATATYPE:
         CHECK_NODECLASS(UA_NODECLASS_VARIABLE | UA_NODECLASS_VARIABLETYPE);