Browse Source

improved version of encoding of enums and opaque types

Julius Pfrommer 10 years ago
parent
commit
b53dad591b

+ 3 - 2
include/ua_types.h

@@ -464,11 +464,12 @@ typedef struct {
     
 struct UA_DataType {
     UA_NodeId typeId; ///< The nodeid of the type
-    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
+    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
     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];
 };

+ 6 - 23
src/server/ua_services_attribute.c

@@ -391,22 +391,12 @@ static UA_StatusCode writeValue(UA_Server *server, UA_WriteValue *wvalue) {
                     break;
                 }
 
-                // array sizes are not compared
-
-                if(!wvalue->value.hasVariant) {
-                    retval = UA_STATUSCODE_BADWRITENOTSUPPORTED;
+                // 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;
                     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) {
@@ -430,18 +420,11 @@ 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) {
+                if(!wvalue->value.hasVariant || !UA_NodeId_equal(&vtn->value.type->typeId,
+                                                                 &wvalue->value.value.type->typeId)) {
                     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) {

+ 59 - 76
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 > UA_TYPES_XMLELEMENT || !p->type->namespaceZero)
+    if(p->type->typeIndex > 24 || !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 <= UA_TYPES_XMLELEMENT;
+    UA_Boolean isBuiltin = (src->type->namespaceZero && src->type->typeIndex <= 24);
 
     if(isArray) {
         encodingByte |= UA_VARIANT_ENCODINGMASKTYPE_ARRAY;
@@ -719,40 +719,42 @@ UA_StatusCode UA_Variant_encodeBinary(UA_Variant const *src, UA_ByteString *dst,
     }
 
     if(isBuiltin)
-        encodingByte |= UA_VARIANT_ENCODINGMASKTYPE_TYPEID_MASK &
-            (UA_Byte)UA_TYPES[src->type->typeIndex].typeId.identifier.numeric;
+        encodingByte |= UA_VARIANT_ENCODINGMASKTYPE_TYPEID_MASK & (UA_Byte)src->type->typeId.identifier.numeric;
     else
-        encodingByte |= UA_VARIANT_ENCODINGMASKTYPE_TYPEID_MASK & (UA_Byte)22; // wrap in an extensionobject
+        encodingByte |= UA_VARIANT_ENCODINGMASKTYPE_TYPEID_MASK & (UA_Byte)22;  // ExtensionObject
 
     UA_StatusCode retval = UA_Byte_encodeBinary(&encodingByte, dst, offset);
 
-    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(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(!isBuiltin) {
-            /* 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);
+            // 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);
+        	}
             UA_Byte eoEncoding = UA_EXTENSIONOBJECT_ENCODINGMASK_BODYISBYTESTRING;
-            retval |= UA_Byte_encodeBinary(&eoEncoding, dst, offset);
+            UA_Byte_encodeBinary(&eoEncoding, dst, offset);
             UA_Int32 eoEncodingLength = UA_calcSizeBinary(src->dataPtr, src->type);
-            retval |= UA_Int32_encodeBinary(&eoEncodingLength, dst, offset);
+            UA_Int32_encodeBinary(&eoEncodingLength, dst, offset);
         }
-        retval |= UA_encodeBinary((void*)ptr, src->type, dst, offset);
-        ptr += memSize;
+        retval |= UA_encodeBinary(src->dataPtr, src->type, dst, offset);
     }
 
     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. Currently,
-   we only support ns0 types (todo: attach typedescriptions to datatypenodes) */
+/* The resulting variant always has the storagetype UA_VARIANT_DATA. */
 UA_StatusCode UA_Variant_decodeBinary(UA_ByteString const *src, size_t *offset, UA_Variant *dst) {
     UA_Variant_init(dst);
     UA_Byte encodingByte;
@@ -761,71 +763,52 @@ UA_StatusCode UA_Variant_decodeBinary(UA_ByteString const *src, size_t *offset,
         return retval;
 
     UA_Boolean isArray = encodingByte & UA_VARIANT_ENCODINGMASKTYPE_ARRAY;
-    /* 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)
+    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)
         return UA_STATUSCODE_BADDECODINGERROR;
 
-    /* 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);
+    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);
             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
     }
-    
-    UA_Boolean hasDimensions = isArray && (encodingByte & UA_VARIANT_ENCODINGMASKTYPE_DIMENSIONS);
-    if(hasDimensions) {
+
+    if(hasDimensions && retval == UA_STATUSCODE_GOOD) {
         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 != UA_STATUSCODE_GOOD) {
-            dst->arrayDimensionsSize = -1; // for deleteMembers
-            UA_Variant_deleteMembers(dst);
-            return retval;
-        }
+        if(retval)
+            dst->arrayLength = -1; // for deleteMembers
     }
-    return UA_STATUSCODE_GOOD;
+
+    dst->type = dataType;
+
+    if(retval)
+        UA_Variant_deleteMembers(dst);
+    return retval;
 }
 
 /* DiagnosticInfo */

+ 2 - 2
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
 

+ 1 - 1
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

+ 1 - 1
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

+ 8 - 5
tools/generate_datatypes.py

@@ -88,6 +88,7 @@ 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())
@@ -122,10 +123,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, " + \
-            ".fixedSize = UA_TRUE, .zeroCopyable = UA_TRUE, " + \
+            ".namespaceZero = " + ("UA_TRUE" if namespace_0 else "UA_FALSE") + \
+            ", .fixedSize = UA_TRUE, .zeroCopyable = UA_TRUE, .isStructure = UA_FALSE, " + \
             ".membersSize = 1,\n\t.members = {{.memberTypeIndex = UA_TYPES_INT32," + \
-            ".namespaceZero = UA_TRUE, .padding = 0, .isArray = UA_FALSE }}, .typeIndex = UA_TYPES_INT32 }"
+            ".namespaceZero = UA_TRUE, .padding = 0, .isArray = UA_FALSE }}, .typeIndex = %s }" % (outname.upper() + "_" + self.name[3:].upper())
 
     def functions_c(self, typeTableName):
         return '''#define %s_new (%s*)UA_Int32_new
@@ -158,8 +159,9 @@ 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, .membersSize = 1,\n\t.members = {{.memberTypeIndex = UA_TYPES_BYTESTRING," + \
-            ".namespaceZero = UA_TRUE, .padding = 0, .isArray = UA_FALSE }}, .typeIndex = UA_TYPES_BYTESTRING }"
+            ".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())
 
     def functions_c(self, typeTableName):
         return '''#define %s_new UA_ByteString_new
@@ -227,6 +229,7 @@ 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: