Services: Remove the in-situ service special case

The savings in memory allocations are not worth the additional
complexity. The services now are uniformly defined.

src/server/ua_server_binary.c

@@ -68,17 +68,10 @@ sendServiceFault(UA_SecureChannel *channel, const UA_ByteString *msg,
     return retval;
 }
 
-typedef enum {
-    UA_SERVICETYPE_NORMAL,
-    UA_SERVICETYPE_INSITU,
-    UA_SERVICETYPE_CUSTOM
-} UA_ServiceType;
-
 static void
 getServicePointers(UA_UInt32 requestTypeId, const UA_DataType **requestType,
                    const UA_DataType **responseType, UA_Service *service,
-                   UA_InSituService *serviceInsitu,
-                   UA_Boolean *requiresSession, UA_ServiceType *serviceType) {
+                   UA_Boolean *requiresSession) {
     switch(requestTypeId) {
     case UA_NS0ID_GETENDPOINTSREQUEST_ENCODING_DEFAULTBINARY:
         *service = (UA_Service)Service_GetEndpoints;
@@ -119,13 +112,11 @@ getServicePointers(UA_UInt32 requestTypeId, const UA_DataType **requestType,
         *requestType = &UA_TYPES[UA_TYPES_CREATESESSIONREQUEST];
         *responseType = &UA_TYPES[UA_TYPES_CREATESESSIONRESPONSE];
         *requiresSession = false;
-        *serviceType = UA_SERVICETYPE_CUSTOM;
         break;
     case UA_NS0ID_ACTIVATESESSIONREQUEST_ENCODING_DEFAULTBINARY:
         *service = NULL; //(UA_Service)Service_ActivateSession;
         *requestType = &UA_TYPES[UA_TYPES_ACTIVATESESSIONREQUEST];
         *responseType = &UA_TYPES[UA_TYPES_ACTIVATESESSIONRESPONSE];
-        *serviceType = UA_SERVICETYPE_CUSTOM;
         break;
     case UA_NS0ID_CLOSESESSIONREQUEST_ENCODING_DEFAULTBINARY:
         *service = (UA_Service)Service_CloseSession;
@@ -134,10 +125,9 @@ getServicePointers(UA_UInt32 requestTypeId, const UA_DataType **requestType,
         break;
     case UA_NS0ID_READREQUEST_ENCODING_DEFAULTBINARY:
         *service = NULL;
-        *serviceInsitu = (UA_InSituService)Service_Read;
+        *service = (UA_Service)Service_Read;
         *requestType = &UA_TYPES[UA_TYPES_READREQUEST];
         *responseType = &UA_TYPES[UA_TYPES_READRESPONSE];
-        *serviceType = UA_SERVICETYPE_INSITU;
         break;
     case UA_NS0ID_WRITEREQUEST_ENCODING_DEFAULTBINARY:
         *service = (UA_Service)Service_Write;
@@ -421,13 +411,11 @@ processMSG(UA_Server *server, UA_SecureChannel *channel,
 
     /* Get the service pointers */
     UA_Service service = NULL;
-    UA_InSituService serviceInsitu = NULL;
     const UA_DataType *requestType = NULL;
     const UA_DataType *responseType = NULL;
     UA_Boolean sessionRequired = true;
-    UA_ServiceType serviceType = UA_SERVICETYPE_NORMAL;
     getServicePointers(requestTypeId.identifier.numeric, &requestType,
-                       &responseType, &service, &serviceInsitu, &sessionRequired, &serviceType);
+                       &responseType, &service, &sessionRequired);
     if(!requestType) {
         if(requestTypeId.identifier.numeric == 787) {
             UA_LOG_INFO_CHANNEL(&server->config.logger, channel,
@@ -496,7 +484,7 @@ processMSG(UA_Server *server, UA_SecureChannel *channel,
                                     requestId, UA_STATUSCODE_BADSESSIONIDINVALID);
         }
         Service_ActivateSession(server, channel, session,
-            (const UA_ActivateSessionRequest*)request,
+                                (const UA_ActivateSessionRequest*)request,
                                 (UA_ActivateSessionResponse*)response);
         goto send_response;
     }
@@ -563,7 +551,10 @@ processMSG(UA_Server *server, UA_SecureChannel *channel,
     }
 #endif
 
-    send_response:
+    /* Dispatch the synchronous service call */
+    service(server, session, request, response);
+
+ send_response:
 
     /* Prepare the ResponseHeader */
     ((UA_ResponseHeader*)response)->requestHandle = requestHeader->requestHandle;
@@ -581,13 +572,6 @@ processMSG(UA_Server *server, UA_SecureChannel *channel,
         }
     }
 
-    /* Process normal services before initializing the message context.
-     * Some services may initialize new message contexts and to support network
-     * layers only providing one send buffer, only one message context can be
-     * initialized concurrently. */
-    if(serviceType == UA_SERVICETYPE_NORMAL)
-        service(server, session, request, response);
-
     /* Start the message */
     UA_NodeId typeId = UA_NODEID_NUMERIC(0, responseType->binaryEncodingId);
     UA_MessageContext mc;
@@ -602,28 +586,9 @@ processMSG(UA_Server *server, UA_SecureChannel *channel,
     retval = UA_MessageContext_encode(&mc, &typeId, &UA_TYPES[UA_TYPES_NODEID]);
     if(retval != UA_STATUSCODE_GOOD)
         goto cleanup;
-
-    switch(serviceType) {
-    case UA_SERVICETYPE_CUSTOM:
-        /* Was processed before...*/
-        retval = UA_MessageContext_encode(&mc, response, responseType);
-        break;
-    case UA_SERVICETYPE_INSITU:
-        retval = serviceInsitu
-            (server, session, &mc, request, (UA_ResponseHeader*)response);
-        break;
-    case UA_SERVICETYPE_NORMAL:
-    default:
-        retval = UA_MessageContext_encode(&mc, response, responseType);
-        break;
-    }
-
-    /* Finish sending the message */
-    if(retval != UA_STATUSCODE_GOOD) {
-        UA_MessageContext_abort(&mc);
+    retval = UA_MessageContext_encode(&mc, response, responseType);
+    if(retval != UA_STATUSCODE_GOOD)
         goto cleanup;
-    }
-
     retval = UA_MessageContext_finish(&mc);
 
  cleanup:
@@ -631,6 +596,7 @@ processMSG(UA_Server *server, UA_SecureChannel *channel,
         UA_LOG_INFO_CHANNEL(&server->config.logger, channel,
                             "Could not send the message over the SecureChannel "
                             "with StatusCode %s", UA_StatusCode_name(retval));
+
     /* Clean up */
     UA_deleteMembers(request, requestType);
     UA_deleteMembers(response, responseType);

src/server/ua_server_internal.h

@@ -175,14 +175,14 @@ UA_Server_writeWithSession(UA_Server *server, UA_Session *session,
 /* Many services come as an array of operations. This function generalizes the
  * processing of the operations. */
 typedef void (*UA_ServiceOperation)(UA_Server *server, UA_Session *session,
-                                    void *context,
+                                    const void *context,
                                     const void *requestOperation,
                                     void *responseOperation);
 
 UA_StatusCode
 UA_Server_processServiceOperations(UA_Server *server, UA_Session *session,
                                    UA_ServiceOperation operationCallback,
-                                   void *context,
+                                   const void *context,
                                    const size_t *requestOperations,
                                    const UA_DataType *requestOperationsType,
                                    size_t *responseOperations,

src/server/ua_server_utils.c

@@ -243,7 +243,7 @@ UA_Server_editNode(UA_Server *server, UA_Session *session,
 UA_StatusCode
 UA_Server_processServiceOperations(UA_Server *server, UA_Session *session,
                                    UA_ServiceOperation operationCallback,
-                                   void *context, const size_t *requestOperations,
+                                   const void *context, const size_t *requestOperations,
                                    const UA_DataType *requestOperationsType,
                                    size_t *responseOperations,
                                    const UA_DataType *responseOperationsType) {

src/server/ua_services.h

@@ -47,9 +47,6 @@ _UA_BEGIN_DECLS
 typedef void (*UA_Service)(UA_Server*, UA_Session*,
                            const void *request, void *response);
 
-typedef UA_StatusCode (*UA_InSituService)(UA_Server*, UA_Session*, UA_MessageContext *mc,
-                                          const void *request, UA_ResponseHeader *rh);
-
 /**
  * Discovery Service Set
  * ---------------------
@@ -305,8 +302,8 @@ void Service_UnregisterNodes(UA_Server *server, UA_Session *session,
  * elements are indexed, such as an array, this Service allows Clients to read
  * the entire set of indexed values as a composite, to read individual elements
  * or to read ranges of elements of the composite. */
-UA_StatusCode Service_Read(UA_Server *server, UA_Session *session, UA_MessageContext *mc,
-                           const UA_ReadRequest *request, UA_ResponseHeader *responseHeader);
+void Service_Read(UA_Server *server, UA_Session *session,
+                  const UA_ReadRequest *request, UA_ReadResponse *response);
 
 /**
  * Write Service
@@ -316,8 +313,7 @@ UA_StatusCode Service_Read(UA_Server *server, UA_Session *session, UA_MessageCon
  * the entire set of indexed values as a composite, to write individual elements
  * or to write ranges of elements of the composite. */
 void Service_Write(UA_Server *server, UA_Session *session,
-                   const UA_WriteRequest *request,
-                   UA_WriteResponse *response);
+                   const UA_WriteRequest *request, UA_WriteResponse *response);
 
 /**
  * HistoryRead Service

src/server/ua_services_attribute.c

@@ -380,79 +380,52 @@ ReadWithNode(const UA_Node *node, UA_Server *server, UA_Session *session,
     }
 }
 
-static UA_StatusCode
-Operation_Read(UA_Server *server, UA_Session *session, UA_MessageContext *mc,
-               UA_TimestampsToReturn timestampsToReturn, const UA_ReadValueId *id) {
-    UA_DataValue dv;
-    UA_DataValue_init(&dv);
-
+static void
+Operation_Read(UA_Server *server, UA_Session *session, UA_ReadRequest *request,
+               UA_ReadValueId *rvi, UA_DataValue *result) {
     /* Get the node */
-    const UA_Node *node = UA_Nodestore_getNode(server->nsCtx, &id->nodeId);
+    const UA_Node *node = UA_Nodestore_getNode(server->nsCtx, &rvi->nodeId);
 
     /* Perform the read operation */
     if(node) {
-        ReadWithNode(node, server, session, timestampsToReturn, id, &dv);
+        ReadWithNode(node, server, session, request->timestampsToReturn, rvi, result);
+        UA_Nodestore_releaseNode(server->nsCtx, node);
     } else {
-        dv.hasStatus = true;
-        dv.status = UA_STATUSCODE_BADNODEIDUNKNOWN;
+        result->hasStatus = true;
+        result->status = UA_STATUSCODE_BADNODEIDUNKNOWN;
     }
-
-    /* Encode (and send) the results */
-    UA_StatusCode retval = UA_MessageContext_encode(mc, &dv, &UA_TYPES[UA_TYPES_DATAVALUE]);
-
-    /* Free copied data and release the node */
-    UA_Variant_deleteMembers(&dv.value);
-    UA_Nodestore_releaseNode(server->nsCtx, node);
-    return retval;
 }
 
-UA_StatusCode Service_Read(UA_Server *server, UA_Session *session, UA_MessageContext *mc,
-                           const UA_ReadRequest *request, UA_ResponseHeader *responseHeader) {
-    UA_LOG_DEBUG_SESSION(&server->config.logger, session,
-                         "Processing ReadRequest");
+void
+Service_Read(UA_Server *server, UA_Session *session,
+             const UA_ReadRequest *request, UA_ReadResponse *response) {
+    UA_LOG_DEBUG_SESSION(&server->config.logger, session, "Processing ReadRequest");
 
     /* Check if the timestampstoreturn is valid */
-    if(request->timestampsToReturn > UA_TIMESTAMPSTORETURN_NEITHER)
-        responseHeader->serviceResult = UA_STATUSCODE_BADTIMESTAMPSTORETURNINVALID;
-
-    if(request->nodesToReadSize == 0)
-        responseHeader->serviceResult = UA_STATUSCODE_BADNOTHINGTODO;
+    if(request->timestampsToReturn < 0 ||
+       request->timestampsToReturn > UA_TIMESTAMPSTORETURN_NEITHER) {
+        response->responseHeader.serviceResult = UA_STATUSCODE_BADTIMESTAMPSTORETURNINVALID;
+        return;
+    }
 
     /* Check if maxAge is valid */
-    if(request->maxAge < 0)
-        responseHeader->serviceResult = UA_STATUSCODE_BADMAXAGEINVALID;
+    if(request->maxAge < 0) {
+        response->responseHeader.serviceResult = UA_STATUSCODE_BADMAXAGEINVALID;
+        return;
+    }
 
     /* Check if there are too many operations */
     if(server->config.maxNodesPerRead != 0 &&
-       request->nodesToReadSize > server->config.maxNodesPerRead)
-        responseHeader->serviceResult = UA_STATUSCODE_BADTOOMANYOPERATIONS;
-
-    /* Encode the response header */
-    UA_StatusCode retval =
-        UA_MessageContext_encode(mc, responseHeader, &UA_TYPES[UA_TYPES_RESPONSEHEADER]);
-    if(retval != UA_STATUSCODE_GOOD)
-        return retval;
-
-    /* Process nothing if we return an error code for the entire service */
-    UA_Int32 arraySize = (UA_Int32)request->nodesToReadSize;
-    if(responseHeader->serviceResult != UA_STATUSCODE_GOOD)
-        arraySize = 0;
-
-    /* Process all ReadValueIds */
-    retval = UA_MessageContext_encode(mc, &arraySize, &UA_TYPES[UA_TYPES_INT32]);
-    if(retval != UA_STATUSCODE_GOOD)
-        return retval;
-
-    for(UA_Int32 i = 0; i < arraySize; i++) {
-        retval = Operation_Read(server, session, mc, request->timestampsToReturn,
-                                &request->nodesToRead[i]);
-        if(retval != UA_STATUSCODE_GOOD)
-            return retval;
+       request->nodesToReadSize > server->config.maxNodesPerRead) {
+        response->responseHeader.serviceResult = UA_STATUSCODE_BADTOOMANYOPERATIONS;
+        return;
     }
 
-    /* Don't return any DiagnosticInfo */
-    arraySize = -1;
-    return UA_MessageContext_encode(mc, &arraySize, &UA_TYPES[UA_TYPES_INT32]);
+    response->responseHeader.serviceResult =
+        UA_Server_processServiceOperations(server, session, (UA_ServiceOperation)Operation_Read,
+                                           request,
+                                           &request->nodesToReadSize, &UA_TYPES[UA_TYPES_READVALUEID],
+                                           &response->resultsSize, &UA_TYPES[UA_TYPES_DATAVALUE]);
 }
 
 UA_DataValue

src/ua_securechannel.c

@@ -889,13 +889,8 @@ UA_MessageContext_encode(UA_MessageContext *mc, const void *content,
                          const UA_DataType *contentType) {
     UA_StatusCode retval = UA_encodeBinary(content, contentType, &mc->buf_pos, &mc->buf_end,
                                            sendSymmetricEncodingCallback, mc);
-    if(retval != UA_STATUSCODE_GOOD) {
-        /* TODO: Send the abort message */
-        if(mc->messageBuffer.length > 0) {
-            UA_Connection *connection = mc->channel->connection;
-            connection->releaseSendBuffer(connection, &mc->messageBuffer);
-        }
-    }
+    if(retval != UA_STATUSCODE_GOOD && mc->messageBuffer.length > 0)
+        UA_MessageContext_abort(mc);
     return retval;
 }
 

tests/server/check_server_readspeed.c

@@ -81,31 +81,33 @@ START_TEST(readSpeed) {
     UA_Byte *pos = request_msg.data;
     const UA_Byte *end = &request_msg.data[request_msg.length];
     retval |= UA_encodeBinary(&request, &UA_TYPES[UA_TYPES_READREQUEST], &pos, &end, NULL, NULL);
-    UA_assert(retval == UA_STATUSCODE_GOOD);
-
-    UA_ReadRequest rq;
-    UA_MessageContext mc;
+    ck_assert(retval == UA_STATUSCODE_GOOD);
 
-    UA_ResponseHeader rh;
-    UA_ResponseHeader_init(&rh);
+    UA_ReadRequest req;
+    UA_ReadResponse res;
+    UA_ReadResponse_init(&res);
 
     clock_t begin, finish;
     begin = clock();
 
     for(size_t i = 0; i < 1000000; i++) {
         size_t offset = 0;
-        retval |= UA_decodeBinary(&request_msg, &offset, &rq, &UA_TYPES[UA_TYPES_READREQUEST], NULL);
+        retval |= UA_decodeBinary(&request_msg, &offset, &req, &UA_TYPES[UA_TYPES_READREQUEST], NULL);
 
-        UA_MessageContext_begin(&mc, &testChannel, 0, UA_MESSAGETYPE_MSG);
-        retval |= Service_Read(server, &server->adminSession, &mc, &rq, &rh);
-        UA_MessageContext_finish(&mc);
+        Service_Read(server, &server->adminSession, &req, &res);
 
-        UA_ReadRequest_deleteMembers(&rq);
+        UA_Byte *rpos = response_msg.data;
+        const UA_Byte *rend = &response_msg.data[response_msg.length];
+        retval |= UA_encodeBinary(&res, &UA_TYPES[UA_TYPES_READRESPONSE],
+                                  &rpos, &rend, NULL, NULL);
+
+        UA_ReadRequest_deleteMembers(&req);
+        UA_ReadResponse_deleteMembers(&res);
     }
 
     finish = clock();
+    ck_assert(retval == UA_STATUSCODE_GOOD);
 
-    UA_assert(retval == UA_STATUSCODE_GOOD);
     double time_spent = (double)(finish - begin) / CLOCKS_PER_SEC;
     printf("duration was %f s\n", time_spent);
     printf("retval is %s\n", UA_StatusCode_name(retval));