2

I am attempting to write multiple nodes in a single request, however I have not found any documentation or examples on how to do that, every time I find anything regarding the issue, a single node is written. Based on my understanding of the open62541 library (which is not much), I've attempted to do this like so:

void Write_from_3_to_5_piece_queue() {
        char NodeID[128];
        char NodeID_backup[128];
        char aux[3];
        bool bool_to_write = false;
        strcpy(NodeID_backup, _BaseNodeID);
        strcat(NodeID_backup, "POU.AT2.piece_queue["); // this is where I want to write, I need only to append the array index in which to write


        UA_WriteRequest wReq;
        UA_WriteValue my_nodes[3]; // this is where I start to make things up, I'm not sure this is the correct way to do it
        my_nodes[0] = *UA_WriteValue_new();
        my_nodes[1] = *UA_WriteValue_new();
        my_nodes[2] = *UA_WriteValue_new();
        strcpy(NodeID, NodeID_backup);
        strcat(NodeID, "3]"); //append third index of array (will write to piece_queue[3])
        my_nodes[0].nodeId = UA_NODEID_STRING_ALLOC(_nodeIndex, NodeID);
        my_nodes[0].attributeId = UA_ATTRIBUTEID_VALUE;
        my_nodes[0].value.hasValue = true;
        my_nodes[0].value.value.type = &UA_TYPES[UA_TYPES_BOOLEAN];
        my_nodes[0].value.value.storageType = UA_VARIANT_DATA_NODELETE;
        my_nodes[0].value.value.data = &bool_to_write;

        strcpy(NodeID, NodeID_backup);
        strcat(NodeID, "4]");
        my_nodes[1].nodeId = UA_NODEID_STRING_ALLOC(_nodeIndex, NodeID);
        my_nodes[1].attributeId = UA_ATTRIBUTEID_VALUE;
        my_nodes[1].value.hasValue = true;
        my_nodes[1].value.value.type = &UA_TYPES[UA_TYPES_BOOLEAN];
        my_nodes[1].value.value.storageType = UA_VARIANT_DATA_NODELETE;
        my_nodes[1].value.value.data = &bool_to_write;

        strcpy(NodeID, NodeID_backup);
        strcat(NodeID, "5]");
        my_nodes[2].nodeId = UA_NODEID_STRING_ALLOC(_nodeIndex, NodeID);
        my_nodes[2].attributeId = UA_ATTRIBUTEID_VALUE;
        my_nodes[2].value.hasValue = true;
        my_nodes[2].value.value.type = &UA_TYPES[UA_TYPES_BOOLEAN];
        my_nodes[2].value.value.storageType = UA_VARIANT_DATA_NODELETE;
        my_nodes[2].value.value.data = &bool_to_write;


        UA_WriteRequest_init(&wReq);
        wReq.nodesToWrite = my_nodes;
        wReq.nodesToWriteSize = 3;
        UA_WriteResponse wResp = UA_Client_Service_write(_client, wReq);
        UA_WriteResponse_clear(&wResp);
        UA_WriteRequest_clear(&wReq);

        return;
    }

At first I didn't have much hope that this would work, but it turns out this actually writes the values that I wish. The problem is that on UA_WriteRequest_clear(&wReq); I trigger an exception in the open62541 library: enter image description here

Also, I know I can write multiple values to arrays specifically, even though in this particular example that would fix my issue, that's not what I mean to do, this example is just to simplify my problem. Just suppose I have a multi-type structure and I want to write to it, all in a single request. I appreciate any help!

C. Pinto
  • 83
  • 4

1 Answers1

3

First of all, this is bad:


UA_WriteValue my_nodes[3];
        my_nodes[0] = *UA_WriteValue_new();
        my_nodes[1] = *UA_WriteValue_new();
        my_nodes[2] = *UA_WriteValue_new();

my_nodes is already created on the stack, and then you are copying the content of a new object into it by dereferencing. This definitely leads to memory leaks. You probably want to use UA_WriteValue_init() instead. Never ever dereference the return value of a new() function.

Let's go bottom up:

UA_WriteRequest_clear(&wReq) is recursively freeing all content of the wReq steucture. This means that it will also call: UA_Array_delete(wReq.nodesToWrite, wReq.nodesToWriteSize, ...) which in turn calls UA_free(wReq.nodesToWrite)

And you have: wReq.nodesToWrite = my_nodes; with UA_WriteValue my_nodes[3];

This means that you are assigning a variable, which lives on the stack to a pointer, and later this pointer is freed. free can only delete stuff which is on the heap and not stack, and therefore it fails.

You have two options now:

  1. If you still want to use the stack trick the UA_clear in thinking that the variable is empty:
wReq.nodesToWrite = NULL;
wReq.nodesToWriteSize = 0;
UA_clear(&wReq);
  1. Put the nodes on the heap: Instead of UA_WriteValue my_nodes[3]; use Something like UA_WriteValue *my_nodes = (UA_WriteValue*)UA_malloc(sizeof(UA_WriteValue)*3);

Also I strongly recommend that you either use valgrind or clang memory sanitizer to avoid all these memory issues.

Stefan Profanter
  • 6,458
  • 6
  • 41
  • 73
  • 1
    Thanks for the quick answer! I'm not completely clear how I could implement option two. Are you saying that UA_WriteValue_new() only allocates memory like you did? I attempted to allocate memory the size of 3 WriteValues to a UA_WriteValue pointer (as you described) but I still get an error on the clear. I've successfully implemented the first option, and tricked the clear by pointing to NULL. My worry is that by using the stack and then tricking the UA_clear into thinking there's no value there might be some leakage that results from UA_WriteValue_init(), do you know if this is the case? – C. Pinto May 08 '20 at 11:12
  • 1
    Simply put, `_clear` tries to free the whole content you pass in recursively. If any of the children is on the stack, this will fail. I strongly recommend that you first make sure that you understand heap and stack, including C pointers, heap allocation and stack allocation. It's very crucial for the open62541 stack. And also try to use clang memory sanitizer. It will tell you exactly, where the error is. – Stefan Profanter May 11 '20 at 07:07
  • Make sure to call `UA_WriteValue_init()` on a pointer to EACH initialized index of `wReq.nodesToWrite`. Otherwise you'll get an error on the clear. For example `wReq.nodesToWrite = (UA_WriteValue*) UA_malloc(2*sizeof(UA_WriteValue)); UA_WriteValue_init(&wReq.nodesToWrite[0]; UA_WriteValue_init(&wReq.nodesToWrite[1];` – Steven T. Snyder Jan 20 '21 at 00:52