Browse Source

always copy data from the data source (removing the release function pointer)

Julius Pfrommer 9 years ago
parent
commit
5caa997870
4 changed files with 61 additions and 177 deletions
  1. 29 85
      examples/server.c
  2. 4 16
      include/ua_server.h
  3. 9 34
      src/server/ua_server.c
  4. 19 42
      src/server/ua_services_attribute.c

+ 29 - 85
examples/server.c

@@ -39,8 +39,7 @@ UA_Logger logger;
 /*************************/
 /* Read-only data source */
 /*************************/
-static UA_StatusCode readTimeData(void *handle, UA_Boolean sourceTimeStamp,
-                                  const UA_NumericRange *range, UA_DataValue *value) {
+static UA_StatusCode readTimeData(void *handle, UA_Boolean sourceTimeStamp, const UA_NumericRange *range, UA_DataValue *value) {
     if(range) {
         value->hasStatus = UA_TRUE;
         value->status = UA_STATUSCODE_BADINDEXRANGEINVALID;
@@ -53,8 +52,6 @@ static UA_StatusCode readTimeData(void *handle, UA_Boolean sourceTimeStamp,
 	value->value.type = &UA_TYPES[UA_TYPES_DATETIME];
 	value->value.arrayLength = -1;
 	value->value.data = currentTime;
-	value->value.arrayDimensionsSize = -1;
-	value->value.arrayDimensions = NULL;
 	value->hasValue = UA_TRUE;
 	if(sourceTimeStamp) {
 		value->hasSourceTimestamp = UA_TRUE;
@@ -63,18 +60,12 @@ static UA_StatusCode readTimeData(void *handle, UA_Boolean sourceTimeStamp,
 	return UA_STATUSCODE_GOOD;
 }
 
-static void releaseTimeData(void *handle, UA_DataValue *value) {
-    if(value->hasValue)
-        UA_DateTime_delete((UA_DateTime*)value->value.data);
-}
-
 /*****************************/
 /* Read-only CPU temperature */
 /*      Only on Linux        */
 /*****************************/
 FILE* temperatureFile = NULL;
-static UA_StatusCode readTemperature(void *handle, UA_Boolean sourceTimeStamp,
-                                     const UA_NumericRange *range, UA_DataValue *value) {
+static UA_StatusCode readTemperature(void *handle, UA_Boolean sourceTimeStamp, const UA_NumericRange *range, UA_DataValue *value) {
     if(range) {
         value->hasStatus = UA_TRUE;
         value->status = UA_STATUSCODE_BADINDEXRANGEINVALID;
@@ -104,11 +95,6 @@ static UA_StatusCode readTemperature(void *handle, UA_Boolean sourceTimeStamp,
 	return UA_STATUSCODE_GOOD;
 }
 
-static void releaseTemperature(void *handle, UA_DataValue *value) {
-    if(value->hasValue)
-        UA_Double_delete((UA_Double*)value->value.data);
-}
-
 /*************************/
 /* Read-write status led */
 /*************************/
@@ -119,25 +105,13 @@ FILE* triggerFile = NULL;
 FILE* ledFile = NULL;
 UA_Boolean ledStatus = 0;
 
-static UA_StatusCode readLedStatus(void *handle, UA_Boolean sourceTimeStamp,
-                                   const UA_NumericRange *range, UA_DataValue *value) {
-    if(range) {
-        value->hasStatus = UA_TRUE;
-        value->status = UA_STATUSCODE_BADINDEXRANGEINVALID;
-        return UA_STATUSCODE_GOOD;
-    }
+static UA_StatusCode readLedStatus(void *handle, UA_Boolean sourceTimeStamp, const UA_NumericRange *range, UA_DataValue *value) {
+    if(range)
+        return UA_STATUSCODE_BADINDEXRANGEINVALID;
 
-	/* In order to reduce blocking time, we could alloc memory for every read
-       and return a copy of the data. */
-#ifdef UA_MULTITHREADING
-	pthread_rwlock_rdlock(&writeLock);
-#endif
-	value->value.type = &UA_TYPES[UA_TYPES_BOOLEAN];
-	value->value.arrayLength = -1;
-	value->value.data = &ledStatus;
-	value->value.arrayDimensionsSize = -1;
-	value->value.arrayDimensions = NULL;
-	value->hasValue = UA_TRUE;
+    UA_StatusCode retval = UA_Variant_setScalarCopy(&value->value, &ledStatus, &UA_TYPES[UA_TYPES_BOOLEAN]);
+    if(retval != UA_STATUSCODE_GOOD)
+        return retval;
 	if(sourceTimeStamp) {
 		value->sourceTimestamp = UA_DateTime_now();
 		value->hasSourceTimestamp = UA_TRUE;
@@ -145,18 +119,6 @@ static UA_StatusCode readLedStatus(void *handle, UA_Boolean sourceTimeStamp,
 	return UA_STATUSCODE_GOOD;
 }
 
-static void releaseLedStatus(void *handle, UA_DataValue *value) {
-    if(!value->hasValue)
-        return;
-	/* If we allocated memory for a specific read, free the content of the
-       variantdata. */
-	value->value.arrayLength = -1;
-	value->value.data = NULL;
-#ifdef UA_MULTITHREADING
-	pthread_rwlock_unlock(&writeLock);
-#endif
-}
-
 static UA_StatusCode writeLedStatus(void *handle, const UA_Variant *data, const UA_NumericRange *range) {
     if(range)
         return UA_STATUSCODE_BADINDEXRANGEINVALID;
@@ -170,12 +132,11 @@ static UA_StatusCode writeLedStatus(void *handle, const UA_Variant *data, const
 	if(triggerFile)
 		fseek(triggerFile, 0, SEEK_SET);
 
-	if(ledFile){
-		if(ledStatus == 1){
+	if(ledFile) {
+		if(ledStatus == 1)
 			fprintf(ledFile, "%s", "1");
-		} else {
+		else
 			fprintf(ledFile, "%s", "0");
-		}
 		fflush(ledFile);
 	}
 #ifdef UA_MULTITHREADING
@@ -192,10 +153,7 @@ static void stopHandler(int sign) {
 static UA_ByteString loadCertificate(void) {
 	UA_ByteString certificate = UA_STRING_NULL;
 	FILE *fp = NULL;
-	//FIXME: a potential bug of locating the certificate, we need to get the path from the server's config
-	fp=fopen("server_cert.der", "rb");
-
-	if(!fp) {
+	if(!(fp=fopen("server_cert.der", "rb"))) {
         errno = 0; // we read errno also from the tcp layer...
         return certificate;
     }
@@ -229,36 +187,24 @@ int main(int argc, char** argv) {
 	UA_Server_addNetworkLayer(server, ServerNetworkLayerTCP_new(UA_ConnectionConfig_standard, 16664));
 
 	// add node with the datetime data source
-	UA_DataSource dateDataSource = (UA_DataSource)
-        {.handle = NULL,
-		.read = readTimeData,
-		.release = releaseTimeData,
-		.write = NULL};
+	UA_DataSource dateDataSource = (UA_DataSource) {.handle = NULL, .read = readTimeData, .write = NULL};
 	const UA_QualifiedName dateName = UA_QUALIFIEDNAME(1, "current time");
 	UA_Server_addDataSourceVariableNode(server, dateDataSource, dateName, UA_NODEID_NULL,
-                                        UA_NODEID_NUMERIC(0, UA_NS0ID_OBJECTSFOLDER),
-                                        UA_NODEID_NUMERIC(0, UA_NS0ID_ORGANIZES));
+                                        UA_NODEID_NUMERIC(0, UA_NS0ID_OBJECTSFOLDER), UA_NODEID_NUMERIC(0, UA_NS0ID_ORGANIZES));
 
 #ifndef _WIN32
 	//cpu temperature monitoring for linux machines
-	if((temperatureFile = fopen("/sys/class/thermal/thermal_zone0/temp", "r"))){
+	if((temperatureFile = fopen("/sys/class/thermal/thermal_zone0/temp", "r"))) {
 		// add node with the data source
-		UA_DataSource temperatureDataSource = (UA_DataSource)
-    	    {.handle = NULL,
-			.read = readTemperature,
-			.release = releaseTemperature,
-			.write = NULL};
+		UA_DataSource temperatureDataSource = (UA_DataSource) {.handle = NULL, .read = readTemperature, .write = NULL};
 		const UA_QualifiedName tempName = UA_QUALIFIEDNAME(1, "cpu temperature");
 		UA_Server_addDataSourceVariableNode(server, temperatureDataSource, tempName, UA_NODEID_NULL,
-                                            UA_NODEID_NUMERIC(0, UA_NS0ID_OBJECTSFOLDER),
-                                            UA_NODEID_NUMERIC(0, UA_NS0ID_ORGANIZES));
+                                            UA_NODEID_NUMERIC(0, UA_NS0ID_OBJECTSFOLDER), UA_NODEID_NUMERIC(0, UA_NS0ID_ORGANIZES));
 	}
 
 	//LED control for rpi
-	if(  access("/sys/class/leds/led0/trigger", F_OK ) != -1
-	  || access("/sys/class/leds/led0/brightness", F_OK ) != -1){
-        if (	(triggerFile = fopen("/sys/class/leds/led0/trigger", "w"))
-            && 	(ledFile = fopen("/sys/class/leds/led0/brightness", "w"))) {
+	if(access("/sys/class/leds/led0/trigger", F_OK ) != -1 || access("/sys/class/leds/led0/brightness", F_OK ) != -1) {
+        if((triggerFile = fopen("/sys/class/leds/led0/trigger", "w")) && (ledFile = fopen("/sys/class/leds/led0/brightness", "w"))) {
             //setting led mode to manual
             fprintf(triggerFile, "%s", "none");
             fflush(triggerFile);
@@ -268,16 +214,12 @@ int main(int argc, char** argv) {
             fflush(ledFile);
 
             // add node with the LED status data source
-            UA_DataSource ledStatusDataSource = (UA_DataSource)
-                {.handle = NULL,
-                .read = readLedStatus,
-                .release = releaseLedStatus,
-                .write = writeLedStatus};
+            UA_DataSource ledStatusDataSource = (UA_DataSource) {.handle = NULL, .read = readLedStatus, .write = writeLedStatus};
             const UA_QualifiedName statusName = UA_QUALIFIEDNAME(0, "status LED");
             UA_Server_addDataSourceVariableNode(server, ledStatusDataSource, statusName, UA_NODEID_NULL,
                                                 UA_NODEID_NUMERIC(0, UA_NS0ID_OBJECTSFOLDER),
                                                 UA_NODEID_NUMERIC(0, UA_NS0ID_ORGANIZES));
-        }else{
+        } else {
             UA_LOG_WARNING(logger, UA_LOGCATEGORY_USERLAND, "[Raspberry Pi] LED file exist, but I have no access (try to run server with sudo)");
         }
     }
@@ -299,13 +241,16 @@ int main(int argc, char** argv) {
    /**************/
 
 #define DEMOID 50000
-   UA_Server_addObjectNode(server,UA_QUALIFIEDNAME(1, "Demo"), UA_NODEID_NUMERIC(1, DEMOID), UA_NODEID_NUMERIC(0, UA_NS0ID_OBJECTSFOLDER), UA_NODEID_NUMERIC(0, UA_NS0ID_ORGANIZES), UA_NODEID_NUMERIC(0, UA_NS0ID_FOLDERTYPE));
+   UA_Server_addObjectNode(server,UA_QUALIFIEDNAME(1, "Demo"), UA_NODEID_NUMERIC(1, DEMOID), UA_NODEID_NUMERIC(0, UA_NS0ID_OBJECTSFOLDER),
+                           UA_NODEID_NUMERIC(0, UA_NS0ID_ORGANIZES), UA_NODEID_NUMERIC(0, UA_NS0ID_FOLDERTYPE));
 
 #define SCALARID 50001
-   UA_Server_addObjectNode(server,UA_QUALIFIEDNAME(1, "Scalar"), UA_NODEID_NUMERIC(1, SCALARID), UA_NODEID_NUMERIC(1, DEMOID), UA_NODEID_NUMERIC(0, UA_NS0ID_ORGANIZES), UA_NODEID_NUMERIC(0, UA_NS0ID_FOLDERTYPE));
+   UA_Server_addObjectNode(server,UA_QUALIFIEDNAME(1, "Scalar"), UA_NODEID_NUMERIC(1, SCALARID), UA_NODEID_NUMERIC(1, DEMOID),
+                           UA_NODEID_NUMERIC(0, UA_NS0ID_ORGANIZES), UA_NODEID_NUMERIC(0, UA_NS0ID_FOLDERTYPE));
 
 #define ARRAYID 50002
-   UA_Server_addObjectNode(server,UA_QUALIFIEDNAME(1, "Array"), UA_NODEID_NUMERIC(1, ARRAYID), UA_NODEID_NUMERIC(1, DEMOID), UA_NODEID_NUMERIC(0, UA_NS0ID_ORGANIZES), UA_NODEID_NUMERIC(0, UA_NS0ID_FOLDERTYPE));
+   UA_Server_addObjectNode(server,UA_QUALIFIEDNAME(1, "Array"), UA_NODEID_NUMERIC(1, ARRAYID), UA_NODEID_NUMERIC(1, DEMOID),
+                           UA_NODEID_NUMERIC(0, UA_NS0ID_ORGANIZES), UA_NODEID_NUMERIC(0, UA_NS0ID_FOLDERTYPE));
 
    UA_UInt32 id = 51000; //running id in namespace 0
    for(UA_UInt32 type = 0; UA_IS_BUILTIN(type); type++) {
@@ -337,16 +282,15 @@ int main(int argc, char** argv) {
 	if(temperatureFile)
 		fclose(temperatureFile);
 
-	if(triggerFile){
+	if(triggerFile) {
 		fseek(triggerFile, 0, SEEK_SET);
 		//setting led mode to default
 		fprintf(triggerFile, "%s", "mmc0");
 		fclose(triggerFile);
 	}
 
-	if(ledFile){
+	if(ledFile)
 		fclose(ledFile);
-	}
 
 #ifdef UA_MULTITHREADING
 	pthread_rwlock_destroy(&writeLock);

+ 4 - 16
include/ua_server.h

@@ -82,18 +82,15 @@ UA_StatusCode UA_EXPORT UA_Server_run_shutdown(UA_Server *server, UA_UInt16 nThr
 UA_StatusCode UA_EXPORT UA_Server_run_mainloop(UA_Server *server, UA_Boolean *running);
 
 /**
- * Datasources are the interface to local data providers. Implementors of datasources need to
- * provide functions for the callbacks in this structure. After every read, the handle needs to be
- * released to indicate that the pointer is no longer accessed. As a rule, datasources are never
- * copied, but only their content. The only way to write into a datasource is via the write-service.
- * It is expected that the read and release callbacks are implemented. The write callback can be set
+ * Datasources are the interface to local data providers. It is expected that
+ * the read and release callbacks are implemented. The write callback can be set
  * to null.
  */
 typedef struct {
     void *handle; ///> A custom pointer to reuse the same datasource functions for multiple sources
 
     /**
-     * Read current data from the data source
+     * Copies the data from the source into the provided value.
      *
      * @param handle An optional pointer to user-defined data for the specific data source
      * @param includeSourceTimeStamp If true, then the datasource is expected to set the source
@@ -105,16 +102,7 @@ typedef struct {
      * @return Returns a status code for logging. Error codes intended for the original caller are set
      *         in the value. If an error is returned, then no releasing of the value is done.
      */
-    UA_StatusCode (*read)(void *handle, UA_Boolean includeSourceTimeStamp, const UA_NumericRange *range,
-                          UA_DataValue *value);
-
-    /**
-     * Release data that was allocated during a read (and/or release locks in the data source)
-     *
-     * @param handle An optional pointer to user-defined data for the specific data source
-     * @param value The DataValue that was used for a successful read.
-     */
-    void (*release)(void *handle, UA_DataValue *value);
+    UA_StatusCode (*read)(void *handle, UA_Boolean includeSourceTimeStamp, const UA_NumericRange *range, UA_DataValue *value);
 
     /**
      * Write into a data source. The write member of UA_DataSource can be empty if the operation

+ 9 - 34
src/server/ua_server.c

@@ -189,13 +189,13 @@ static void getBulidInfo(const UA_Server* server, UA_BuildInfo *buildInfo) {
     buildInfo->buildDate = server->buildDate;
 }
 
-static UA_StatusCode readStatus(void *handle, UA_Boolean sourceTimeStamp,
-                                const UA_NumericRange *range, UA_DataValue *value) {
+static UA_StatusCode readStatus(void *handle, UA_Boolean sourceTimeStamp, const UA_NumericRange *range, UA_DataValue *value) {
     if(range) {
         value->hasStatus = UA_TRUE;
         value->status = UA_STATUSCODE_BADINDEXRANGEINVALID;
         return UA_STATUSCODE_GOOD;
     }
+    
     UA_ServerStatusDataType *status = UA_ServerStatusDataType_new();
     status->startTime   = ((const UA_Server*)handle)->startTime;
     status->currentTime = UA_DateTime_now();
@@ -216,28 +216,17 @@ static UA_StatusCode readStatus(void *handle, UA_Boolean sourceTimeStamp,
     return UA_STATUSCODE_GOOD;
 }
 
-static void releaseStatus(void *handle, UA_DataValue *value) {
-    if(!value->hasValue)
-        return;
-    UA_ServerStatusDataType_delete((UA_ServerStatusDataType*)value->value.data);
-    value->value.data = UA_NULL;
-    value->hasValue = UA_FALSE;
-    UA_DataValue_deleteMembers(value);
-}
-
-static UA_StatusCode readNamespaces(void *handle, UA_Boolean sourceTimestamp,
-                                    const UA_NumericRange *range, UA_DataValue *value) {
+static UA_StatusCode readNamespaces(void *handle, UA_Boolean sourceTimestamp, const UA_NumericRange *range, UA_DataValue *value) {
     if(range) {
         value->hasStatus = UA_TRUE;
         value->status = UA_STATUSCODE_BADINDEXRANGEINVALID;
         return UA_STATUSCODE_GOOD;
     }
     UA_Server *server = (UA_Server*)handle;
+    UA_StatusCode retval = UA_Variant_setArrayCopy(&value->value, server->namespaces, server->namespacesSize, &UA_TYPES[UA_TYPES_STRING]);
+    if(retval != UA_STATUSCODE_GOOD)
+        return retval;
     value->hasValue = UA_TRUE;
-    value->value.storageType = UA_VARIANT_DATA_NODELETE;
-    value->value.type = &UA_TYPES[UA_TYPES_STRING];
-    value->value.arrayLength = server->namespacesSize;
-    value->value.data = server->namespaces;
     if(sourceTimestamp) {
         value->hasSourceTimestamp = UA_TRUE;
         value->sourceTimestamp = UA_DateTime_now();
@@ -245,9 +234,6 @@ static UA_StatusCode readNamespaces(void *handle, UA_Boolean sourceTimestamp,
     return UA_STATUSCODE_GOOD;
 }
 
-static void releaseNamespaces(void *handle, UA_DataValue *value) {
-}
-
 static UA_StatusCode readCurrentTime(void *handle, UA_Boolean sourceTimeStamp,
                                      const UA_NumericRange *range, UA_DataValue *value) {
     if(range) {
@@ -269,11 +255,6 @@ static UA_StatusCode readCurrentTime(void *handle, UA_Boolean sourceTimeStamp,
     return UA_STATUSCODE_GOOD;
 }
 
-static void releaseCurrentTime(void *handle, UA_DataValue *value) {
-    if(value->hasValue)
-        UA_DateTime_delete((UA_DateTime*)value->value.data);
-}
-
 static void copyNames(UA_Node *node, char *name) {
     node->browseName = UA_QUALIFIEDNAME_ALLOC(0, name);
     node->displayName = UA_LOCALIZEDTEXT_ALLOC("", name);
@@ -835,9 +816,7 @@ UA_Server * UA_Server_new(UA_ServerConfig config) {
    copyNames((UA_Node*)namespaceArray, "NamespaceArray");
    namespaceArray->nodeId.identifier.numeric = UA_NS0ID_SERVER_NAMESPACEARRAY;
    namespaceArray->valueSource = UA_VALUESOURCE_DATASOURCE;
-   namespaceArray->value.dataSource = (UA_DataSource) {
-       .handle = server, .read = readNamespaces,
-       .release = releaseNamespaces, .write = UA_NULL};
+   namespaceArray->value.dataSource = (UA_DataSource) {.handle = server, .read = readNamespaces, .write = UA_NULL};
    namespaceArray->valueRank = 1;
    namespaceArray->minimumSamplingInterval = 1.0;
    UA_Server_addNode(server, (UA_Node*)namespaceArray, UA_EXPANDEDNODEID_NUMERIC(0, UA_NS0ID_SERVER),
@@ -953,9 +932,7 @@ UA_Server * UA_Server_new(UA_ServerConfig config) {
       copyNames((UA_Node*)serverstatus, "ServerStatus");
       serverstatus->nodeId = UA_NODEID_NUMERIC(0, UA_NS0ID_SERVER_SERVERSTATUS);
       serverstatus->valueSource = UA_VALUESOURCE_DATASOURCE;
-      serverstatus->value.dataSource = (UA_DataSource) {
-          .handle = server, .read = readStatus,
-          .release = releaseStatus, .write = UA_NULL};
+      serverstatus->value.dataSource = (UA_DataSource) {.handle = server, .read = readStatus, .write = UA_NULL};
       UA_Server_addNode(server, (UA_Node*)serverstatus, UA_EXPANDEDNODEID_NUMERIC(0, UA_NS0ID_SERVER),
                         nodeIdHasComponent);
       UA_Server_addReference(server, UA_NODEID_NUMERIC(0, UA_NS0ID_SERVER_SERVERSTATUS),
@@ -976,9 +953,7 @@ UA_Server * UA_Server_new(UA_ServerConfig config) {
       copyNames((UA_Node*)currenttime, "CurrentTime");
       currenttime->nodeId = UA_NODEID_NUMERIC(0, UA_NS0ID_SERVER_SERVERSTATUS_CURRENTTIME);
       currenttime->valueSource = UA_VALUESOURCE_DATASOURCE;
-      currenttime->value.dataSource = (UA_DataSource) {
-          .handle = NULL, .read = readCurrentTime,
-          .release = releaseCurrentTime, .write = UA_NULL};
+      currenttime->value.dataSource = (UA_DataSource) {.handle = NULL, .read = readCurrentTime, .write = UA_NULL};
       UA_Server_addNode(server, (UA_Node*)currenttime,
                         UA_EXPANDEDNODEID_NUMERIC(0, UA_NS0ID_SERVER_SERVERSTATUS),
                         nodeIdHasComponent);

+ 19 - 42
src/server/ua_services_attribute.c

@@ -198,18 +198,18 @@ void readValue(UA_Server *server, UA_TimestampsToReturn timestamps,
     			handleSourceTimestamps(timestamps, v);
             }
 
-            UA_Boolean hasRange = UA_FALSE;
             UA_NumericRange range;
+            UA_NumericRange *rangeptr = UA_NULL;
             if(id->indexRange.length > 0) {
                 retval = parse_numericrange(id->indexRange, &range);
                 if(retval != UA_STATUSCODE_GOOD)
                     break;
-                hasRange = UA_TRUE;
+                rangeptr = ⦥
             }
 
             const UA_VariableNode *vn = (const UA_VariableNode*)node;
             if(vn->valueSource == UA_VALUESOURCE_VARIANT) {
-                if(hasRange)
+                if(rangeptr)
                     retval |= UA_Variant_copyRange(&vn->value.variant, &v->value, range);
                 else
                     retval |= UA_Variant_copy(&vn->value.variant, &v->value);
@@ -218,21 +218,11 @@ void readValue(UA_Server *server, UA_TimestampsToReturn timestamps,
                     handleSourceTimestamps(timestamps, v);
                 }
             } else {
-                UA_DataValue val;
-                UA_DataValue_init(&val);
-                UA_Boolean sourceTimeStamp = (timestamps == UA_TIMESTAMPSTORETURN_SOURCE ||
-                                              timestamps == UA_TIMESTAMPSTORETURN_BOTH);
-                if(hasRange)
-                    retval |= vn->value.dataSource.read(vn->value.dataSource.handle, sourceTimeStamp, &range, &val);
-                else
-                    retval |= vn->value.dataSource.read(vn->value.dataSource.handle, sourceTimeStamp, UA_NULL, &val);
-                if(retval == UA_STATUSCODE_GOOD) {
-                    retval |= UA_DataValue_copy(&val, v); // todo: still too much copying necessary!!
-                    vn->value.dataSource.release(vn->value.dataSource.handle, &val);
-                }
+                UA_Boolean sourceTimeStamp = (timestamps == UA_TIMESTAMPSTORETURN_SOURCE || timestamps == UA_TIMESTAMPSTORETURN_BOTH);
+                retval |= vn->value.dataSource.read(vn->value.dataSource.handle, sourceTimeStamp, rangeptr, v);
             }
 
-            if(hasRange)
+            if(rangeptr)
                 UA_free(range.dimensions);
         }
         break;
@@ -241,8 +231,7 @@ void readValue(UA_Server *server, UA_TimestampsToReturn timestamps,
 		CHECK_NODECLASS(UA_NODECLASS_VARIABLE | UA_NODECLASS_VARIABLETYPE);
         const UA_VariableNode *vn = (const UA_VariableNode*)node;
         if(vn->valueSource == UA_VALUESOURCE_VARIANT)
-            retval = UA_Variant_setScalarCopy(&v->value, &vn->value.variant.type->typeId,
-                                              &UA_TYPES[UA_TYPES_NODEID]);
+            retval = UA_Variant_setScalarCopy(&v->value, &vn->value.variant.type->typeId, &UA_TYPES[UA_TYPES_NODEID]);
         else {
             UA_DataValue val;
             UA_DataValue_init(&val);
@@ -250,8 +239,9 @@ void readValue(UA_Server *server, UA_TimestampsToReturn timestamps,
             if(retval != UA_STATUSCODE_GOOD)
                 break;
             retval = UA_Variant_setScalarCopy(&v->value, &val.value.type->typeId, &UA_TYPES[UA_TYPES_NODEID]);
-            vn->value.dataSource.release(vn->value.dataSource.handle, &val);
+            UA_DataValue_deleteMembers(&val);
         }
+
         if(retval == UA_STATUSCODE_GOOD)
             v->hasValue = UA_TRUE;
         }
@@ -260,8 +250,7 @@ void readValue(UA_Server *server, UA_TimestampsToReturn timestamps,
     case UA_ATTRIBUTEID_VALUERANK:
         CHECK_NODECLASS(UA_NODECLASS_VARIABLE | UA_NODECLASS_VARIABLETYPE);
         v->hasValue = UA_TRUE;
-        retval |= UA_Variant_setScalarCopy(&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:
@@ -269,9 +258,7 @@ void readValue(UA_Server *server, UA_TimestampsToReturn timestamps,
         {
             const UA_VariableNode *vn = (const UA_VariableNode *)node;
             if(vn->valueSource == UA_VALUESOURCE_VARIANT) {
-                retval = UA_Variant_setArrayCopy(&v->value, vn->value.variant.arrayDimensions,
-                                                 vn->value.variant.arrayDimensionsSize,
-                                                 &UA_TYPES[UA_TYPES_INT32]);
+                retval = UA_Variant_setArrayCopy(&v->value, vn->value.variant.arrayDimensions, vn->value.variant.arrayDimensionsSize, &UA_TYPES[UA_TYPES_INT32]);
                 if(retval == UA_STATUSCODE_GOOD)
                     v->hasValue = UA_TRUE;
             } else {
@@ -280,12 +267,8 @@ void readValue(UA_Server *server, UA_TimestampsToReturn timestamps,
                 retval |= vn->value.dataSource.read(vn->value.dataSource.handle, UA_FALSE, UA_NULL, &val);
                 if(retval != UA_STATUSCODE_GOOD)
                     break;
-                if(!val.hasValue)
-                    retval = UA_STATUSCODE_BADNOTREADABLE;
-                else
-                    retval = UA_Variant_setArrayCopy(&v->value, val.value.arrayDimensions,
-                                                     val.value.arrayDimensionsSize, &UA_TYPES[UA_TYPES_INT32]);
-                vn->value.dataSource.release(vn->value.dataSource.handle, &val);
+                retval = UA_Variant_setArrayCopy(&v->value, val.value.arrayDimensions, val.value.arrayDimensionsSize, &UA_TYPES[UA_TYPES_INT32]);
+                UA_DataValue_deleteMembers(&val);
             }
         }
         break;
@@ -293,43 +276,37 @@ void readValue(UA_Server *server, UA_TimestampsToReturn timestamps,
     case UA_ATTRIBUTEID_ACCESSLEVEL:
         CHECK_NODECLASS(UA_NODECLASS_VARIABLE);
         v->hasValue = UA_TRUE;
-        retval |= UA_Variant_setScalarCopy(&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);
         v->hasValue = UA_TRUE;
-        retval |= UA_Variant_setScalarCopy(&v->value, &((const UA_VariableNode *)node)->userAccessLevel,
-                                          &UA_TYPES[UA_TYPES_BYTE]);
+        retval |= UA_Variant_setScalarCopy(&v->value, &((const UA_VariableNode *)node)->userAccessLevel, &UA_TYPES[UA_TYPES_BYTE]);
         break;
 
     case UA_ATTRIBUTEID_MINIMUMSAMPLINGINTERVAL:
         CHECK_NODECLASS(UA_NODECLASS_VARIABLE);
         v->hasValue = UA_TRUE;
-        retval |= UA_Variant_setScalarCopy(&v->value, &((const UA_VariableNode *)node)->minimumSamplingInterval,
-                                          &UA_TYPES[UA_TYPES_DOUBLE]);
+        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);
         v->hasValue = UA_TRUE;
-        retval |= UA_Variant_setScalarCopy(&v->value, &((const UA_VariableNode *)node)->historizing,
-                                          &UA_TYPES[UA_TYPES_BOOLEAN]);
+        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);
         v->hasValue = UA_TRUE;
-        retval |= UA_Variant_setScalarCopy(&v->value, &((const UA_MethodNode *)node)->executable,
-                                          &UA_TYPES[UA_TYPES_BOOLEAN]);
+        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);
         v->hasValue = UA_TRUE;
-        retval |= UA_Variant_setScalarCopy(&v->value, &((const UA_MethodNode *)node)->userExecutable,
-                                          &UA_TYPES[UA_TYPES_BOOLEAN]);
+        retval |= UA_Variant_setScalarCopy(&v->value, &((const UA_MethodNode *)node)->userExecutable, &UA_TYPES[UA_TYPES_BOOLEAN]);
         break;
 
     default: