Incorrect Memory Allocation

[j-medland]

I opened a PR #372 to address a memory allocation error in the function below

grpc-labview/src/grpc_interop.cc

Lines 26 to 38 in 4026aa8

	void OccurServerEvent(LVUserEventRef event, gRPCid* data, std::string eventMethodName)
	{
	LStr* lvMethodName = (LStr*)malloc(sizeof(int32_t) + eventMethodName.length() + 1);
	lvMethodName->cnt = eventMethodName.length();
	memcpy(lvMethodName->str, eventMethodName.c_str(), eventMethodName.length());
	GeneralMethodEventData eventData;
	eventData.methodData = data;
	eventData.methodName = &lvMethodName;
	auto error = PostUserEvent(event, &eventData);
	free(lvMethodName);
	}

I thought I would expand here what the issue is and if you would like to specify a particular approach to fixing it then I would be able to submit a new PR.

The issue is whilst copying a std::string into a LabVIEW String you are not allocating the correct number of bytes for a LabVIEW String on either 32 or 64 bit systems.

On a 32-bit system the first 4 bytes of an LStr in memory will contain the string length and the remaining bytes will store the characters of the string - there is no need for a null termination byte to be accounted for so allocating sizeof(int32_t) + string.length() + 1 makes a buffer which is one byte too large which I appreciate is not a huge problem but still unnecessary. You can also just use string.data() instead of string.c_str() for the copy.

On a 64-bit system the first 4 bytes of an LStr in memory will be the size of the string, then there will be 4 padding bytes for memory alignment, then the remaining bytes will store the characters of the string. So your memory allocation is now 3 bytes short!

There are multiple options to fix this.

If you want to use a malloc and free then you could use sizeof(LStr) + string.length() - 1 to determine the number of bytes to allocate.

This works because the definition of LStr as

    struct LStr {
        int32_t cnt; /* number of bytes that follow */
        char str[1]; /* cnt bytes */
    };

... means that any padding between cnt and the first string character byte is accounted for, so you just have to add the number of bytes in the string minus one byte to get the total require memory size.

An alternative is to use NumericArrayResize to allocate the memory, as this handles different system bitness and you already have a function which will allocate and copy a std::string which I mention in my PR #372 .

The only part missing for this approach is to provide a DSDisposeHandle for Linux based systems.

If you think I have made a mistake in my analysis, please let me know.

Cheers

John

AB#2850320