Browse Source

check_builtin with no leaks on valgrind

Julius Pfrommer 10 years ago
parent
commit
7f2db60652
5 changed files with 64 additions and 98 deletions
  1. 17 5
      src/ua_types.c
  2. 1 0
      src/ua_types.h
  3. 0 2
      src/ua_types_encoding_binary.c
  4. 45 90
      tests/check_builtin.c
  5. 1 1
      tests/check_memory.c

+ 17 - 5
src/ua_types.c

@@ -76,6 +76,7 @@ UA_Int32 UA_String_deleteMembers(UA_String *p) {
 	return retval;
 }
 UA_Int32 UA_String_copy(UA_String const *src, UA_String *dst) {
+	if(src == UA_NULL || dst == UA_NULL) return UA_ERROR;
 	UA_Int32 retval = UA_SUCCESS;
 	dst->data   = UA_NULL;
 	dst->length = -1;
@@ -256,8 +257,8 @@ UA_Int32 UA_Guid_init(UA_Guid *p) {
 
 UA_TYPE_NEW_DEFAULT(UA_Guid)
 UA_Int32 UA_Guid_copy(UA_Guid const *src, UA_Guid *dst) {
+	if(src == UA_NULL || dst == UA_NULL) return UA_ERROR;
 	UA_Int32 retval = UA_SUCCESS;
-	retval |= UA_alloc((void **)&dst, sizeof(UA_Guid));
 	retval |= UA_memcpy((void *)dst, (void *)src, sizeof(UA_Guid));
 	return retval;
 }
@@ -399,6 +400,7 @@ UA_Int32 UA_NodeId_init(UA_NodeId *p) {
 
 UA_TYPE_NEW_DEFAULT(UA_NodeId)
 UA_Int32 UA_NodeId_copy(UA_NodeId const *src, UA_NodeId *dst) {
+	if(src == UA_NULL || dst == UA_NULL) return UA_ERROR;
 	UA_Int32 retval = UA_SUCCESS;
 	retval |= UA_Byte_copy(&src->encodingByte, &dst->encodingByte);
 
@@ -476,6 +478,7 @@ UA_Int32 UA_ExpandedNodeId_init(UA_ExpandedNodeId *p) {
 
 UA_TYPE_NEW_DEFAULT(UA_ExpandedNodeId)
 UA_Int32 UA_ExpandedNodeId_copy(UA_ExpandedNodeId const *src, UA_ExpandedNodeId *dst) {
+	if(src == UA_NULL || dst == UA_NULL) return UA_ERROR;
 	UA_Int32 retval = UA_SUCCESS;
 	UA_String_copy(&src->namespaceUri, &dst->namespaceUri);
 	UA_NodeId_copy(&src->nodeId, &dst->nodeId);
@@ -508,8 +511,8 @@ UA_Int32 UA_QualifiedName_init(UA_QualifiedName *p) {
 
 UA_TYPE_NEW_DEFAULT(UA_QualifiedName)
 UA_Int32 UA_QualifiedName_copy(UA_QualifiedName const *src, UA_QualifiedName *dst) {
+	if(src == UA_NULL || dst == UA_NULL) return UA_ERROR;
 	UA_Int32 retval = UA_SUCCESS;
-	retval |= UA_alloc((void **)&dst, sizeof(UA_QualifiedName));
 	retval |= UA_String_copy(&src->name, &dst->name);
 	retval |= UA_UInt16_copy(&src->namespaceIndex, &dst->namespaceIndex);
 	return retval;
@@ -550,8 +553,8 @@ UA_Int32 UA_LocalizedText_copycstring(char const *src, UA_LocalizedText *dst) {
 }
 
 UA_Int32 UA_LocalizedText_copy(UA_LocalizedText const *src, UA_LocalizedText *dst) {
+	if(src == UA_NULL || dst == UA_NULL) return UA_ERROR;
 	UA_Int32 retval = UA_SUCCESS;
-	retval |= UA_alloc((void **)&dst, sizeof(UA_LocalizedText));
 	retval |= UA_Byte_copy(&src->encodingMask, &dst->encodingMask);
 	retval |= UA_String_copy(&src->locale, &dst->locale);
 	retval |= UA_String_copy(&src->text, &dst->text);
@@ -578,6 +581,7 @@ UA_Int32 UA_ExtensionObject_init(UA_ExtensionObject *p) {
 
 UA_TYPE_NEW_DEFAULT(UA_ExtensionObject)
 UA_Int32 UA_ExtensionObject_copy(UA_ExtensionObject const *src, UA_ExtensionObject *dst) {
+	if(src == UA_NULL || dst == UA_NULL) return UA_ERROR;
 	UA_Int32 retval = UA_SUCCESS;
 	retval |= UA_Byte_copy(&src->encoding, &dst->encoding);
 	retval |= UA_ByteString_copy(&src->body, &dst->body);
@@ -608,8 +612,8 @@ UA_Int32 UA_DataValue_init(UA_DataValue *p) {
 
 UA_TYPE_NEW_DEFAULT(UA_DataValue)
 UA_Int32 UA_DataValue_copy(UA_DataValue const *src, UA_DataValue *dst) {
+	if(src == UA_NULL || dst == UA_NULL) return UA_ERROR;
 	UA_Int32 retval = UA_SUCCESS;
-	//TODO can be optimized by direct UA_memcpy call
 	UA_Byte_copy(&src->encodingMask, &dst->encodingMask);
 	UA_Int16_copy(&src->serverPicoseconds, &dst->serverPicoseconds);
 	UA_DateTime_copy(&src->serverTimestamp, &dst->serverTimestamp);
@@ -625,8 +629,12 @@ UA_TYPE_DELETE_DEFAULT(UA_Variant)
 UA_Int32 UA_Variant_deleteMembers(UA_Variant *p) {
 	UA_Int32 retval = UA_SUCCESS;
 	if(p == UA_NULL) return retval;
+
 	if(p->data != UA_NULL) {
-		retval |= UA_Array_delete(p->data, p->arrayLength, UA_ns0ToVTableIndex(&p->vt->typeId));
+		if(p->encodingMask & UA_VARIANT_ENCODINGMASKTYPE_ARRAY)
+			retval |= UA_Array_delete(p->data, p->arrayLength, UA_ns0ToVTableIndex(&p->vt->typeId));
+		else
+			retval |= p->vt->delete(p->data);
 		p->data = UA_NULL;
 	}
 	if(p->arrayDimensions != UA_NULL) {
@@ -649,6 +657,7 @@ UA_Int32 UA_Variant_init(UA_Variant *p) {
 
 UA_TYPE_NEW_DEFAULT(UA_Variant)
 UA_Int32 UA_Variant_copy(UA_Variant const *src, UA_Variant *dst) {
+	if(src == UA_NULL || dst == UA_NULL) return UA_ERROR;
 	UA_Int32  retval = UA_SUCCESS;
 	// Variants are always with types from ns0 or an extensionobject.
 	UA_NodeId typeId =
@@ -720,6 +729,7 @@ UA_Int32 UA_DiagnosticInfo_deleteMembers(UA_DiagnosticInfo *p) {
 	if(p == UA_NULL) return retval;
 	if((p->encodingMask & UA_DIAGNOSTICINFO_ENCODINGMASK_INNERDIAGNOSTICINFO) && p->innerDiagnosticInfo != UA_NULL) {
 		retval |= UA_DiagnosticInfo_delete(p->innerDiagnosticInfo);
+		retval |= UA_String_deleteMembers(&p->additionalInfo);
 		p->innerDiagnosticInfo = UA_NULL;
 	}
 	return retval;
@@ -740,6 +750,7 @@ UA_Int32 UA_DiagnosticInfo_init(UA_DiagnosticInfo *p) {
 
 UA_TYPE_NEW_DEFAULT(UA_DiagnosticInfo)
 UA_Int32 UA_DiagnosticInfo_copy(UA_DiagnosticInfo const *src, UA_DiagnosticInfo *dst) {
+	if(src == UA_NULL || dst == UA_NULL) return UA_ERROR;
 	UA_Int32 retval = UA_SUCCESS;
 	retval |= UA_String_copy(&src->additionalInfo, &dst->additionalInfo);
 	retval |= UA_Byte_copy(&src->encodingMask, &dst->encodingMask);
@@ -839,6 +850,7 @@ UA_Int32 UA_Array_init(void *p, UA_Int32 noElements, UA_Int32 type) {
 }
 
 UA_Int32 UA_Array_copy(const void *src, UA_Int32 noElements, UA_Int32 type, void **dst) {
+	if(src == UA_NULL || dst == UA_NULL) return UA_ERROR;
 	if(UA_VTable_isValidType(type) != UA_SUCCESS)
 		return UA_ERROR;
 

+ 1 - 0
src/ua_types.h

@@ -432,6 +432,7 @@ typedef struct UA_VTable {
 /* Use only when the type has no arrays. Otherwise, implement deep copy */
 #define UA_TYPE_COPY_DEFAULT(TYPE)                     \
     UA_Int32 TYPE##_copy(TYPE const *src, TYPE *dst) { \
+    	if(src == UA_NULL || dst == UA_NULL) return UA_ERROR; \
 		UA_Int32 retval = UA_SUCCESS;                  \
 		retval |= UA_memcpy(dst, src, sizeof(TYPE));   \
 		return retval;                                 \

+ 0 - 2
src/ua_types_encoding_binary.c

@@ -741,13 +741,11 @@ UA_Int32 UA_Variant_decodeBinary(UA_ByteString const *src, UA_UInt32 *offset, UA
 		dst->arrayLength = -1;
 	} else {
 		// allocate array and decode
-		CHECKED_DECODE(UA_Array_new(&dst->data, dst->arrayLength, uaIdx), dst->data = UA_NULL);
 		CHECKED_DECODE(UA_Array_decodeBinary(src, offset, dst->arrayLength, uaIdx, &dst->data), UA_Variant_deleteMembers(dst));
 	}
 	//decode the dimension field array if present
 	if(dst->encodingMask & UA_VARIANT_ENCODINGMASKTYPE_DIMENSIONS) {
 		UA_Int32_decodeBinary(src, offset, &dst->arrayDimensionsLength);
-		CHECKED_DECODE(UA_Array_new((void **)&dst->arrayDimensions, dst->arrayDimensionsLength, UA_INT32), dst->arrayDimensions = UA_NULL);
 		CHECKED_DECODE(UA_Array_decodeBinary(src, offset, dst->arrayLength, uaIdx,
 		                                     &dst->data), UA_Variant_deleteMembers(dst));
 	}

+ 45 - 90
tests/check_builtin.c

@@ -1458,88 +1458,46 @@ END_TEST
 
 START_TEST(UA_DiagnosticInfo_copyShallWorkOnExample) {
 	//given
-	UA_DiagnosticInfo *value = UA_NULL;
-	UA_DiagnosticInfo *innerValue  = UA_NULL;
-	UA_DiagnosticInfo *copiedValue = UA_NULL;
-	UA_String testString;
-	UA_Int32  size = 5;
-	UA_Int32  i    = 0;
-	testString.data = UA_NULL;
+	UA_DiagnosticInfo value, innerValue, copiedValue;
+	UA_String testString = (UA_String){5, (UA_Byte*)"OPCUA"};
 
-	UA_alloc((void **)&testString.data, size);
-	testString.data[0] = 'O';
-	testString.data[1] = 'P';
-	testString.data[2] = 'C';
-	testString.data[3] = 'U';
-	testString.data[4] = 'A';
-	testString.length  = size;
-
-	UA_DiagnosticInfo_new(&value);
-	UA_DiagnosticInfo_new(&innerValue);
-	value->encodingMask |= UA_DIAGNOSTICINFO_ENCODINGMASK_INNERDIAGNOSTICINFO;
-	value->innerDiagnosticInfo = innerValue;
+	UA_DiagnosticInfo_init(&value);
+	UA_DiagnosticInfo_init(&innerValue);
+	value.encodingMask |= UA_DIAGNOSTICINFO_ENCODINGMASK_INNERDIAGNOSTICINFO;
+	value.innerDiagnosticInfo = &innerValue;
+	value.additionalInfo = testString;
 
-	UA_alloc((void **)&copiedValue, UA_DiagnosticInfo_calcSizeBinary(UA_NULL));
-	value->additionalInfo.length = testString.length;
-	value->additionalInfo.data   = testString.data;
 	//when
-	UA_DiagnosticInfo_copy(value, copiedValue);
+	UA_DiagnosticInfo_copy(&value, &copiedValue);
 
 	//then
-	for(i = 0;i < size;i++)
-		ck_assert_int_eq(copiedValue->additionalInfo.data[i], value->additionalInfo.data[i]);
-	ck_assert_int_eq(copiedValue->additionalInfo.length, value->additionalInfo.length);
-
-	ck_assert_int_eq(copiedValue->encodingMask, value->encodingMask);
-	ck_assert_int_eq(copiedValue->innerDiagnosticInfo->locale, value->innerDiagnosticInfo->locale);
-	ck_assert_int_eq(copiedValue->innerStatusCode, value->innerStatusCode);
-	ck_assert_int_eq(copiedValue->locale, value->locale);
-	ck_assert_int_eq(copiedValue->localizedText, value->localizedText);
-	ck_assert_int_eq(copiedValue->namespaceUri, value->namespaceUri);
-	ck_assert_int_eq(copiedValue->symbolicId, value->symbolicId);
+	for(UA_Int32 i = 0;i < testString.length;i++)
+		ck_assert_int_eq(copiedValue.additionalInfo.data[i], value.additionalInfo.data[i]);
+	ck_assert_int_eq(copiedValue.additionalInfo.length, value.additionalInfo.length);
+
+	ck_assert_int_eq(copiedValue.encodingMask, value.encodingMask);
+	ck_assert_int_eq(copiedValue.innerDiagnosticInfo->locale, value.innerDiagnosticInfo->locale);
+	ck_assert_int_eq(copiedValue.innerStatusCode, value.innerStatusCode);
+	ck_assert_int_eq(copiedValue.locale, value.locale);
+	ck_assert_int_eq(copiedValue.localizedText, value.localizedText);
+	ck_assert_int_eq(copiedValue.namespaceUri, value.namespaceUri);
+	ck_assert_int_eq(copiedValue.symbolicId, value.symbolicId);
+
 	//finally
-	UA_free(copiedValue);
-	UA_free(value);
+	value.additionalInfo.data = UA_NULL; // do not delete the static string
+	value.innerDiagnosticInfo = UA_NULL; // do not delete the static innerdiagnosticinfo
+	UA_DiagnosticInfo_deleteMembers(&value);
+	UA_DiagnosticInfo_deleteMembers(&copiedValue);
 
 }
 END_TEST
 START_TEST(UA_ApplicationDescription_copyShallWorkOnExample) {
 	//given
 
-	UA_Int32  retval = 0;
-	UA_String appString;
-	UA_String discString;
-	UA_String gateWayString;
-	UA_Int32  appSize     = 3;
-	UA_Int32  discSize    = 4;
-	UA_Int32  gateWaySize = 7;
-	UA_Int32  i, j;
-	appString.data     = UA_NULL;
-	discString.data    = UA_NULL;
-	gateWayString.data = UA_NULL;
-
-	UA_alloc((void **)&appString.data, appSize);
-	appString.data[0] = 'A';
-	appString.data[1] = 'P';
-	appString.data[2] = 'P';
-	appString.length  = appSize;
-
-	UA_alloc((void **)&discString.data, discSize);
-	discString.data[0] = 'D';
-	discString.data[1] = 'I';
-	discString.data[2] = 'S';
-	discString.data[3] = 'C';
-	discString.length  = discSize;
-
-	UA_alloc((void **)&gateWayString.data, gateWaySize);
-	gateWayString.data[0] = 'G';
-	gateWayString.data[1] = 'A';
-	gateWayString.data[2] = 'T';
-	gateWayString.data[3] = 'E';
-	gateWayString.data[4] = 'W';
-	gateWayString.data[5] = 'A';
-	gateWayString.data[6] = 'Y';
-	gateWayString.length  = gateWaySize;
+	UA_Int32  retval = UA_SUCCESS;
+	UA_String appString = (UA_String){3, (UA_Byte*)"APP"};
+	UA_String discString = (UA_String){4, (UA_Byte*)"DISC"};
+	UA_String gateWayString = (UA_String){7, (UA_Byte*)"GATEWAY"};
 
 	UA_String srcArray[3];
 	srcArray[0] = (UA_String){ 6, (UA_Byte*)"__open" };
@@ -1548,12 +1506,9 @@ START_TEST(UA_ApplicationDescription_copyShallWorkOnExample) {
 
 	UA_ApplicationDescription value, copiedValue;
 	UA_ApplicationDescription_init(&value);
-	value.applicationUri.length = appString.length;
-	value.applicationUri.data   = appString.data;
-	value.discoveryProfileUri.length = discString.length;
-	value.discoveryProfileUri.data   = discString.data;
-	value.gatewayServerUri.length    = gateWayString.length;
-	value.gatewayServerUri.data      = gateWayString.data;
+	value.applicationUri = appString;
+	value.discoveryProfileUri = discString;
+	value.gatewayServerUri = gateWayString;
 	value.discoveryUrlsSize = 3;
 	value.discoveryUrls     = srcArray;
 
@@ -1563,47 +1518,44 @@ START_TEST(UA_ApplicationDescription_copyShallWorkOnExample) {
 	//then
 	ck_assert_int_eq(retval, UA_SUCCESS);
 
-	for(i = 0;i < appSize;i++)
+	for(UA_Int32 i = 0;i < appString.length;i++)
 		ck_assert_int_eq(copiedValue.applicationUri.data[i], value.applicationUri.data[i]);
 	ck_assert_int_eq(copiedValue.applicationUri.length, value.applicationUri.length);
 
-	for(i = 0;i < discSize;i++)
+	for(UA_Int32 i = 0;i < discString.length;i++)
 		ck_assert_int_eq(copiedValue.discoveryProfileUri.data[i], value.discoveryProfileUri.data[i]);
 	ck_assert_int_eq(copiedValue.discoveryProfileUri.length, value.discoveryProfileUri.length);
 
-	for(i = 0;i < gateWaySize;i++)
+	for(UA_Int32 i = 0;i < gateWayString.length;i++)
 		ck_assert_int_eq(copiedValue.gatewayServerUri.data[i], value.gatewayServerUri.data[i]);
 	ck_assert_int_eq(copiedValue.gatewayServerUri.length, value.gatewayServerUri.length);
 
 	//String Array Test
-	for(i = 0;i < 3;i++) {
-		for(j = 0;j < 6;j++)
+	for(UA_Int32 i = 0;i < 3;i++) {
+		for(UA_Int32 j = 0;j < 6;j++)
 			ck_assert_int_eq(value.discoveryUrls[i].data[j], copiedValue.discoveryUrls[i].data[j]);
 		ck_assert_int_eq(value.discoveryUrls[i].length, copiedValue.discoveryUrls[i].length);
 	}
 	ck_assert_int_eq(copiedValue.discoveryUrls[0].data[2], 'o');
 	ck_assert_int_eq(copiedValue.discoveryUrls[0].data[3], 'p');
 	ck_assert_int_eq(copiedValue.discoveryUrlsSize, value.discoveryUrlsSize);
+
+	//finally
+	// UA_ApplicationDescription_deleteMembers(&value); // do not free the members as they are statically allocated
+	UA_ApplicationDescription_deleteMembers(&copiedValue);
 }
 END_TEST
 
 START_TEST(UA_Variant_copyShallWorkOnSingleValueExample) {
 	//given
-	UA_String testString;
-	testString.length  = 5;
-	UA_alloc((void **)&testString.data, testString.length);
-	testString.data[0] = 'O';
-	testString.data[1] = 'P';
-	testString.data[2] = 'C';
-	testString.data[3] = 'U';
-	testString.data[4] = 'A';
-
+	UA_String testString = (UA_String){5, (UA_Byte*)"OPCUA"};
 	UA_Variant value, copiedValue;
 	UA_Variant_init(&value);
 	UA_Variant_init(&copiedValue);
 	UA_alloc((void**)&value.data, sizeof(UA_String));
 	*((UA_String*)value.data) = testString;
 	value.encodingMask = UA_STRING_NS0;
+	value.vt = &UA_.types[UA_STRING];
 
 	//when
 	UA_Variant_copy(&value, &copiedValue);
@@ -1619,6 +1571,7 @@ START_TEST(UA_Variant_copyShallWorkOnSingleValueExample) {
 	ck_assert_int_eq(value.arrayLength, copiedValue.arrayLength);
 
 	//finally
+	((UA_String*)value.data)->data = UA_NULL; // the string is statically allocated. do not free it.
 	UA_Variant_deleteMembers(&value);
 	UA_Variant_deleteMembers(&copiedValue);
 }
@@ -1646,6 +1599,7 @@ START_TEST(UA_Variant_copyShallWorkOn1DArrayExample) {
 	value.arrayDimensions       = dimensions;
 	value.encodingMask          = UA_VARIANT_ENCODINGMASKTYPE_ARRAY | UA_STRING_NS0 |
 	                              UA_VARIANT_ENCODINGMASKTYPE_DIMENSIONS;
+	value.vt = &UA_.types[UA_STRING];
 
 	//when
 	UA_Variant_copy(&value, &copiedValue);
@@ -1864,6 +1818,7 @@ int main(void) {
 
 	s  = testSuite_builtin();
 	sr = srunner_create(s);
+	//srunner_set_fork_status(sr, CK_NOFORK);
 	srunner_run_all(sr, CK_NORMAL);
 	number_failed += srunner_ntests_failed(sr);
 	srunner_free(sr);

+ 1 - 1
tests/check_memory.c

@@ -163,7 +163,7 @@ int main() {
 
 	sr = srunner_create(s);
 	//for debugging puposes only, will break make check
-	srunner_set_fork_status(sr, CK_NOFORK);
+	// srunner_set_fork_status(sr, CK_NOFORK);
 	srunner_run_all (sr, CK_NORMAL);
 	number_failed += srunner_ntests_failed(sr);
 	srunner_free(sr);