Revert "improved version of encoding of enums and opaque types"

This reverts commit b53dad591bbb703f8a5f28e6744bec30423cda1e, reversing
changes made to ced600ccdc25c105c4a93a5cb845d49499e12f30.

include/ua_types.h

@@ -464,12 +464,11 @@ typedef struct {
     
 struct UA_DataType {
     UA_NodeId typeId; ///< The nodeid of the type
-    size_t memSize UA_BITFIELD(16); ///< Size of the struct in memory
-    size_t typeIndex UA_BITFIELD(12); ///< Index of the type in the datatypetable
+    ptrdiff_t memSize UA_BITFIELD(16); ///< Size of the struct in memory
+    UA_UInt16 typeIndex UA_BITFIELD(13); ///< Index of the type in the datatypetable
     UA_Boolean namespaceZero UA_BITFIELD(1); ///< The type is defined in namespace zero.
     UA_Boolean fixedSize UA_BITFIELD(1); ///< The type (and its members) contains no pointers
     UA_Boolean zeroCopyable UA_BITFIELD(1); ///< Can the type be copied directly off the stream?
-    UA_Boolean isStructure UA_BITFIELD(1); ///< strcture or not
     UA_Byte membersSize; ///< How many members does the type have?
     UA_DataTypeMember members[UA_MAX_TYPE_MEMBERS];
 };

src/server/ua_services_attribute.c

@@ -391,12 +391,22 @@ static UA_StatusCode writeValue(UA_Server *server, UA_WriteValue *wvalue) {
                     break;
                 }
 
-                // array sizes are not checked to match
-                if(!wvalue->value.hasVariant || !UA_NodeId_equal(&vn->variable.variant.type->typeId,
-                                                                 &wvalue->value.value.type->typeId)) {
-                	retval = UA_STATUSCODE_BADTYPEMISMATCH;
+                // array sizes are not compared
+
+                if(!wvalue->value.hasVariant) {
+                    retval = UA_STATUSCODE_BADWRITENOTSUPPORTED;
                     break;
                 }
+                if(!UA_NodeId_equal(&vn->variable.variant.type->typeId, &wvalue->value.value.type->typeId)) {
+                    // when the nodeids differ, it is possible that an enum was sent as an int, or
+                    // an opaque type as a bytestring
+                    if(!vn->variable.variant.type->namespaceZero || wvalue->value.value.type->namespaceZero ||
+                       vn->variable.variant.type->typeIndex != wvalue->value.value.type->typeIndex) {
+                        retval = UA_STATUSCODE_BADTYPEMISMATCH;
+                        break;
+                    }
+                    wvalue->value.value.type = vn->variable.variant.type;
+                }
 
                 UA_VariableNode *newVn = UA_VariableNode_new();
                 if(!newVn) {
@@ -420,11 +430,18 @@ static UA_StatusCode writeValue(UA_Server *server, UA_WriteValue *wvalue) {
                     UA_VariableNode_delete(newVn);
             } else if(node->nodeClass == UA_NODECLASS_VARIABLETYPE) {
                 const UA_VariableTypeNode *vtn = (const UA_VariableTypeNode*)node;
-                if(!wvalue->value.hasVariant || !UA_NodeId_equal(&vtn->value.type->typeId,
-                                                                 &wvalue->value.value.type->typeId)) {
+                if(!wvalue->value.hasVariant) {
                     retval = UA_STATUSCODE_BADWRITENOTSUPPORTED;
                     break;
                 }
+                if(!UA_NodeId_equal(&vtn->value.type->typeId, &wvalue->value.value.type->typeId)) {
+                    if(!vtn->value.type->namespaceZero || wvalue->value.value.type->namespaceZero ||
+                       vtn->value.type->typeIndex != wvalue->value.value.type->typeIndex) {
+                        retval = UA_STATUSCODE_BADTYPEMISMATCH;
+                        break;
+                    }
+                    wvalue->value.value.type = vtn->value.type;
+                }
 
                 UA_VariableTypeNode *newVtn = UA_VariableTypeNode_new();
                 if(!newVtn) {

src/ua_types_encoding_binary.c

@@ -694,7 +694,7 @@ size_t UA_Variant_calcSizeBinary(UA_Variant const *p) {
         length += UA_Array_calcSizeBinary(p->dataPtr, arrayLength, p->type);
         
     // if the type is not builtin, we encode it as an extensionobject
-    if(p->type->typeIndex > 24 || !p->type->namespaceZero)
+    if(p->type->typeIndex > UA_TYPES_XMLELEMENT || !p->type->namespaceZero)
         length += 9 * arrayLength;  // overhead for extensionobjects: 4 byte nodeid + 1 byte
                                     // encoding + 4 byte bytestring length
     if(arrayLength != 1 && p->arrayDimensions != UA_NULL)
@@ -710,7 +710,7 @@ UA_StatusCode UA_Variant_encodeBinary(UA_Variant const *src, UA_ByteString *dst,
     UA_Byte encodingByte = 0;
     UA_Boolean isArray = src->arrayLength != 1;  // a single element is not an array
     UA_Boolean hasDimensions = isArray && src->arrayDimensions != UA_NULL;
-    UA_Boolean isBuiltin = (src->type->namespaceZero && src->type->typeIndex <= 24);
+    UA_Boolean isBuiltin = src->type->namespaceZero && src->type->typeIndex <= UA_TYPES_XMLELEMENT;
 
     if(isArray) {
         encodingByte |= UA_VARIANT_ENCODINGMASKTYPE_ARRAY;
@@ -719,42 +719,40 @@ UA_StatusCode UA_Variant_encodeBinary(UA_Variant const *src, UA_ByteString *dst,
     }
 
     if(isBuiltin)
-        encodingByte |= UA_VARIANT_ENCODINGMASKTYPE_TYPEID_MASK & (UA_Byte)src->type->typeId.identifier.numeric;
+        encodingByte |= UA_VARIANT_ENCODINGMASKTYPE_TYPEID_MASK &
+            (UA_Byte)UA_TYPES[src->type->typeIndex].typeId.identifier.numeric;
     else
-        encodingByte |= UA_VARIANT_ENCODINGMASKTYPE_TYPEID_MASK & (UA_Byte)22;  // ExtensionObject
+        encodingByte |= UA_VARIANT_ENCODINGMASKTYPE_TYPEID_MASK & (UA_Byte)22; // wrap in an extensionobject
 
     UA_StatusCode retval = UA_Byte_encodeBinary(&encodingByte, dst, offset);
 
-    if(isArray)
-        retval |= UA_Array_encodeBinary(src->dataPtr, src->arrayLength, src->type, dst, offset);
-    else if(!src->dataPtr)
-        retval = UA_STATUSCODE_BADENCODINGERROR; // an array can be empty. a single element must be present.
-    else {
+    if(src->arrayLength != 1)
+        retval |= UA_Int32_encodeBinary(&src->arrayLength, dst, offset);
+    uintptr_t ptr = (uintptr_t)src->dataPtr;
+    ptrdiff_t memSize = src->type->memSize;
+    for(UA_Int32 i=0;i<src->arrayLength;i++) {
         if(!isBuiltin) {
-            // print the extensionobject header
-        	if(src->type->isStructure == UA_TRUE){ //increase id by UA_ENCODINGOFFSET_BINARY for binary encoding of struct obejcts
-        		UA_NodeId copy;
-        		UA_NodeId_copy(&src->type->typeId, &copy);
-        		copy.identifier.numeric=copy.identifier.numeric + UA_ENCODINGOFFSET_BINARY;
-        		UA_NodeId_encodeBinary(&copy, dst, offset);
-        	} else {
-        		UA_NodeId_encodeBinary(&src->type->typeId, dst, offset);
-        	}
+            /* The type is wrapped inside an extensionobject*/
+            UA_NodeId copy;
+            retval |= UA_NodeId_copy(&src->type->typeId, &copy);
+            copy.identifier.numeric += UA_ENCODINGOFFSET_BINARY; // todo: this holds only for namespace 0
+            retval |= UA_NodeId_encodeBinary(&copy, dst, offset);
             UA_Byte eoEncoding = UA_EXTENSIONOBJECT_ENCODINGMASK_BODYISBYTESTRING;
-            UA_Byte_encodeBinary(&eoEncoding, dst, offset);
+            retval |= UA_Byte_encodeBinary(&eoEncoding, dst, offset);
             UA_Int32 eoEncodingLength = UA_calcSizeBinary(src->dataPtr, src->type);
-            UA_Int32_encodeBinary(&eoEncodingLength, dst, offset);
+            retval |= UA_Int32_encodeBinary(&eoEncodingLength, dst, offset);
         }
-        retval |= UA_encodeBinary(src->dataPtr, src->type, dst, offset);
+        retval |= UA_encodeBinary((void*)ptr, src->type, dst, offset);
+        ptr += memSize;
     }
 
     if(hasDimensions)
-        retval |= UA_Array_encodeBinary(src->arrayDimensions, src->arrayDimensionsSize,
-                                        &UA_TYPES[UA_TYPES_INT32], dst, offset);
+        retval |= UA_Array_encodeBinary(src->arrayDimensions, src->arrayDimensionsSize, &UA_TYPES[UA_TYPES_INT32], dst, offset);
     return retval;
 }
 
-/* The resulting variant always has the storagetype UA_VARIANT_DATA. */
+/* The resulting variant always has the storagetype UA_VARIANT_DATA. Currently,
+   we only support ns0 types (todo: attach typedescriptions to datatypenodes) */
 UA_StatusCode UA_Variant_decodeBinary(UA_ByteString const *src, size_t *offset, UA_Variant *dst) {
     UA_Variant_init(dst);
     UA_Byte encodingByte;
@@ -763,52 +761,71 @@ UA_StatusCode UA_Variant_decodeBinary(UA_ByteString const *src, size_t *offset,
         return retval;
 
     UA_Boolean isArray = encodingByte & UA_VARIANT_ENCODINGMASKTYPE_ARRAY;
-    UA_Boolean hasDimensions = isArray && (encodingByte & UA_VARIANT_ENCODINGMASKTYPE_DIMENSIONS);
-    UA_NodeId typeid = { .namespaceIndex = 0, .identifierType = UA_NODEIDTYPE_NUMERIC,
-                         .identifier.numeric = encodingByte & UA_VARIANT_ENCODINGMASKTYPE_TYPEID_MASK };
-
-    UA_UInt16 typeIndex;
-    for(typeIndex = 0; typeIndex < UA_TYPES_COUNT; typeIndex++) {
-        if(UA_TYPES[typeIndex].typeId.identifier.numeric == typeid.identifier.numeric)
-            break;
-    }
-
-    if(typeIndex >= UA_TYPES_COUNT)
+    /* The datatype in the encodingByte is always builtin. Structures are encoded inside an extensionobject */
+    size_t typeIndex = (encodingByte & UA_VARIANT_ENCODINGMASKTYPE_TYPEID_MASK) - 1;
+    if(typeIndex > 24)
         return UA_STATUSCODE_BADDECODINGERROR;
 
-    const UA_DataType *dataType = &UA_TYPES[typeIndex];
-
-    if(!isArray) {
-        if(!(dst->dataPtr = UA_malloc(dataType->memSize)))
-            return UA_STATUSCODE_BADOUTOFMEMORY;
-        retval |= UA_decodeBinary(src, offset, dst->dataPtr, dataType);
-        if(retval) {
-            UA_free(dst->dataPtr);
+    /* Arrays of extensionobjects may contain different types in every element. So we don't
+       transparently decode the content. */
+    const UA_DataType *dataType = UA_NULL;
+    if(isArray || typeIndex != UA_TYPES_EXTENSIONOBJECT) {
+        /* array or builtin */
+        dataType = &UA_TYPES[typeIndex];
+        UA_Int32 arraySize = 1;
+        if(isArray) {
+            retval |= UA_Int32_decodeBinary(src, offset, &arraySize);
+            if(retval != UA_STATUSCODE_GOOD)
+                return retval;
+        }
+        retval |= UA_Array_decodeBinary(src, offset, arraySize, &dst->dataPtr, dataType);
+        if(retval != UA_STATUSCODE_GOOD)
             return retval;
+        dst->arrayLength = arraySize; // for deleteMembers
+        dst->type = dataType;
+    } else {
+        /* single extensionobject */
+        size_t oldoffset = *offset;
+        UA_NodeId typeId;
+        if((retval |= UA_NodeId_decodeBinary(src, offset, &typeId)) != UA_STATUSCODE_GOOD)
+            return retval;
+        UA_Byte EOencodingByte;
+        if((retval |= UA_Byte_decodeBinary(src, offset, &EOencodingByte)) != UA_STATUSCODE_GOOD) {
+            UA_NodeId_deleteMembers(&typeId);
+            return retval;
+        }
+        if(typeId.namespaceIndex == 0 && EOencodingByte == UA_EXTENSIONOBJECT_ENCODINGMASK_BODYISXML) {
+            for(typeIndex = 0;typeIndex < UA_TYPES_COUNT; typeIndex++) {
+                if(UA_NodeId_equal(&typeId, &UA_TYPES[typeIndex].typeId)) {
+                    dataType = &UA_TYPES[typeIndex];
+                    break;
+                }
+            }
+        }
+        UA_NodeId_deleteMembers(&typeId);
+        if(!dataType) {
+            *offset = oldoffset;
+            dataType = &UA_TYPES[UA_TYPES_EXTENSIONOBJECT];
         }
+        if((retval |= UA_decodeBinary(src, offset, dst->dataPtr, dataType)) != UA_STATUSCODE_GOOD)
+            return retval;
+        dst->type = dataType;
         dst->arrayLength = 1;
-    } else {
-        retval |= UA_Int32_decodeBinary(src, offset, &dst->arrayLength);
-        if(retval == UA_STATUSCODE_GOOD)
-            retval |= UA_Array_decodeBinary(src, offset, dst->arrayLength, &dst->dataPtr, dataType);
-        if(retval)
-            dst->arrayLength = -1; // for deleteMembers
     }
-
-    if(hasDimensions && retval == UA_STATUSCODE_GOOD) {
+    
+    UA_Boolean hasDimensions = isArray && (encodingByte & UA_VARIANT_ENCODINGMASKTYPE_DIMENSIONS);
+    if(hasDimensions) {
         retval |= UA_Int32_decodeBinary(src, offset, &dst->arrayDimensionsSize);
         if(retval == UA_STATUSCODE_GOOD)
             retval |= UA_Array_decodeBinary(src, offset, dst->arrayDimensionsSize,
                                             &dst->dataPtr, &UA_TYPES[UA_TYPES_INT32]);
-        if(retval)
-            dst->arrayLength = -1; // for deleteMembers
+        if(retval != UA_STATUSCODE_GOOD) {
+            dst->arrayDimensionsSize = -1; // for deleteMembers
+            UA_Variant_deleteMembers(dst);
+            return retval;
+        }
     }
-
-    dst->type = dataType;
-
-    if(retval)
-        UA_Variant_deleteMembers(dst);
-    return retval;
+    return UA_STATUSCODE_GOOD;
 }
 
 /* DiagnosticInfo */

tests/check_builtin.c

@@ -575,8 +575,8 @@ START_TEST(UA_Double_decodeShallGive2147483648) {
 	// then
 	ck_assert_int_eq(retval, UA_STATUSCODE_GOOD);
 	ck_assert_int_eq(pos, 8);
-	ck_assert(2147483647.9999999 <= dst);
-	ck_assert(dst <= 2147483648.00000001);
+	ck_assert(2147483647.9999999 < dst);
+	ck_assert(dst < 2147483648.00000001);
 }
 END_TEST
 

tools/.coverity.sh

@@ -6,7 +6,7 @@
 #
 
 git fetch origin coverity_scan
-COMMITS=`git log --oneline --since today.midnight | wc -l`
+COMMITS=`git log --oneline --since=today.midnight | wc -l`
 if [[ "$COMMITS" -le "1" ]]; then
     #first commit a day - push changes to branch coverity_scan
     git clone -b coverity_scan https://$GITAUTH@github.com/acplt/open62541

tools/.deployDoxygen.sh

@@ -6,7 +6,7 @@
 #
 
 git fetch origin coverity_scan
-COMMITS=`git log --oneline --since today.midnight | wc -l`
+COMMITS=`git log --oneline --since=today.midnight | wc -l`
 if [[ "$COMMITS" -le "1" ]]; then
    git clone --depth=50 -b gh-pages https://$GITAUTH@github.com/acplt/open62541-www
    cd open62541-www

tools/generate_datatypes.py

@@ -88,7 +88,6 @@ class BuiltinType(object):
             ".namespaceZero = UA_TRUE, " + \
             ".fixedSize = " + ("UA_TRUE" if self.fixed_size() else "UA_FALSE") + \
             ", .zeroCopyable = " + ("UA_TRUE" if self.zero_copy() else "UA_FALSE") + \
-            ", .isStructure = UA_FALSE" + \
             ", .membersSize = 1,\n\t.members = {{.memberTypeIndex = UA_TYPES_" + self.name[3:].upper() + "," + \
             ".namespaceZero = UA_TRUE, .padding = 0, .isArray = UA_FALSE }}, " + \
             ".typeIndex = %s }" % (outname.upper() + "_" + self.name[3:].upper())
@@ -123,10 +122,10 @@ class EnumerationType(object):
             typeid = "{.namespaceIndex = %s, .identifierType = UA_NODEIDTYPE_NUMERIC, .identifier.numeric = %s}, " % (description.namespaceid, description.nodeid)
         return "{.typeId = " + typeid + \
             ".memSize = sizeof(" + self.name + "), " +\
-            ".namespaceZero = " + ("UA_TRUE" if namespace_0 else "UA_FALSE") + \
-            ", .fixedSize = UA_TRUE, .zeroCopyable = UA_TRUE, .isStructure = UA_FALSE, " + \
+            ".namespaceZero = UA_TRUE, " + \
+            ".fixedSize = UA_TRUE, .zeroCopyable = UA_TRUE, " + \
             ".membersSize = 1,\n\t.members = {{.memberTypeIndex = UA_TYPES_INT32," + \
-            ".namespaceZero = UA_TRUE, .padding = 0, .isArray = UA_FALSE }}, .typeIndex = %s }" % (outname.upper() + "_" + self.name[3:].upper())
+            ".namespaceZero = UA_TRUE, .padding = 0, .isArray = UA_FALSE }}, .typeIndex = UA_TYPES_INT32 }"
 
     def functions_c(self, typeTableName):
         return '''#define %s_new (%s*)UA_Int32_new
@@ -159,9 +158,8 @@ class OpaqueType(object):
             typeid = "{.namespaceIndex = %s, .identifierType = UA_NODEIDTYPE_NUMERIC, .identifier.numeric = %s}, " % (description.namespaceid, description.nodeid)
         return "{.typeId = " + typeid + \
             ".memSize = sizeof(" + self.name + "), .fixedSize = UA_FALSE, .zeroCopyable = UA_FALSE, " + \
-            ".namespaceZero = " + ("UA_TRUE" if namespace_0 else "UA_FALSE") + \
-            ", .membersSize = 1,\n\t.members = {{.memberTypeIndex = UA_TYPES_BYTESTRING," + \
-            ".namespaceZero = UA_TRUE, .padding = 0, .isArray = UA_FALSE }}, .typeIndex = %s }" % (outname.upper() + "_" + self.name[3:].upper())
+            ".namespaceZero = UA_TRUE, .membersSize = 1,\n\t.members = {{.memberTypeIndex = UA_TYPES_BYTESTRING," + \
+            ".namespaceZero = UA_TRUE, .padding = 0, .isArray = UA_FALSE }}, .typeIndex = UA_TYPES_BYTESTRING }"
 
     def functions_c(self, typeTableName):
         return '''#define %s_new UA_ByteString_new
@@ -229,7 +227,6 @@ class StructType(object):
                  ", .fixedSize = " + ("UA_TRUE" if self.fixed_size() else "UA_FALSE") + \
                  ", .zeroCopyable = " + ("sizeof(" + self.name + ") == " + str(self.mem_size()) if self.zero_copy() \
                                          else "UA_FALSE") + \
-                 ", .isStructure = UA_TRUE" + \
                  ", .typeIndex = " + outname.upper() + "_" + self.name[3:].upper() + \
                  ", .membersSize = " + str(len(self.members)) + ","
         if len(self.members) > 0: