fix arrayWrite for arrays of strings #165
Conversation
| @@ -1077,24 +1077,25 @@ DataElementUaSdk::writeArray (const char **value, const epicsUInt32 len, | |||
| char *val = nullptr; | |||
| const char *pval; | |||
| // add zero termination if necessary | |||
| if (memchr(value[i], '\0', len) == nullptr) { | |||
| if (memrchr(value, '\0', len) == nullptr) { | |||
There was a problem hiding this comment.
The use of memrchr is more efficient than memchr because usually the last byte already matches 0. However not all Windows Versions seem to support it. I have no problem compiling it with VS 2019 but the CI fails.
There was a problem hiding this comment.
memrchr is a GNU extension (https://www.gnu.org/software/libc/manual/html_node/Search-Functions.html). I am surprised that the MSVC build on your local system supports it.
There was a problem hiding this comment.
Please use memchr on Windows.
Could be a Win-only #define memrchr memchr, couldn't it?
There was a problem hiding this comment.
Äh, memrchr() is not the same as memchr().
If you have trailing crap after a good string, then they may return differnent
values.
Unless someone measures the performance, and says it is 50% faster
using memrchr(), I would vote for using memchr():
a) it is more readable
b) modern processors/compilers do already optimize these mem() operations.
Or do I miss something ?
There was a problem hiding this comment.
I found the reason why it did not trigger in my builds: I have no UaSDK for Windows, so I build the UaSDK Version only for Linux and the open62541 code does not need it.
About Efficiency: Probability that the first byte is 0 is close to 0 (only for empty strings). Probability that the last byte is 0 is 100% (I found no way to put an unterminated string into a string array.) Just to be on the save side, simply checking the last byte is sufficient. Any 0 byte before will be handled by strncpy correctly, even in the very unlikely case of "crap" in the last byte.
There was a problem hiding this comment.
Given how rare arrays of strings are in EPICS... the time savings could be up to milliseconds per year! (Across all EPICS-OPCUA users, of course.)
There was a problem hiding this comment.
So rare that even the tests did not cover them ;-)
dirk-zimoch
force-pushed
the
fix_arrayWrite_strings
branch
from
726bfdf to
8ca3dc3
Compare
|
The builds fail again with some "bad decrypt" exception. I am giving up on this stuff with "secrets" and will from now on ignore any CI failures in this project on my branches. @ralphlange, please check with your setup if it works. |
|
Thanks! The check looks much better now. |
The situation is as before: See https://github.com/dirk-zimoch/opcua/actions I will check if I can selectively suppress PR builds of the UA SDK. |
|
Ah, right. I saw the failure here and thought "not again". Everything looks ok here: https://github.com/dirk-zimoch/opcua/actions |
Writing arrays of strings (waveform or aao records with
FTVL="STRING") wrote only the first element correctly and then just rubbish, possibly crashimg. The reason is that the code used achar**assuming achar*[num]array of pointers to strings but arrays of strings are actuallychar[num][len], i.e. one memory block with all the strings with achar*to the beginning.