search

paulscherrerinstitute / StreamDevice

EPICS Driver for message based I/O
GNU General Public License v3.0
28 stars 42 forks source link

ensure \0 after moving data to start of the buffer #98

Closed hinxx closed 3 months ago

hinxx commented 4 months ago

DO NOT MERGE

This is some debugging done for #97 that might help ..

These ugly code hacks solve the case where newlen is larger than len and the buffer data is moved to the start of the buffer (L350 - L355 in the patch) and there is no \0 seen afterwards. The IOC would always report %s mismatch. In other words, I do not see the mismatch on %s occurring anymore as a consequence of executing this part of the code.

But ..

In the attached file you can find two cases of failure and one success case ; in all cases the newlen+offs == cap and therefore memset() is not called. In the hexdump output one can see 16 more bytes that do not belong to the buffer (cap = 64). I thought byte 40 would be \0 in the successful case ; but that is not the case. Also looking at the lines with buffer after: the next character that causes the %s to fail is not the one at hexdump offset 40. This makes me think that there is another issue elsewhere, outside the StreamBuffer::replace().

stream-errors.txt

Of course I might be totally wrong 'cause I do not know any parts of the source code. I'll be looking into it later on .. unless you beat me to it ;)

EDIT: Ups, just noticed that the text file was not made with master branch of the streamdevice code so the line numbers in there are off. I did run the master branch code, too, and saw the issues I was seeing with the "older" code I was running earlier. Will run all new debug on the master branch code from now on

hinxx commented 4 months ago

After a bit more debugging I think I know what is going on.

In my case the string can vary in size between 31 .. 35 bytes. Sometimes a pair of strings of length 33 and 31 are received in succession from the device. This causes the StreamBuffer::local to be completely filled (StreamBuffer::len is 0x1f). Later on, in scanValue(), when the scanString() starts parsing the field out it consumes A03\x1f. This causes the scanValue() to return -1 and 'Input "A03" does not match format "%s"' error is seen.

As a POC, the second commit introduces a dummy member StreamBuffer::term right after StreamBuffer::local and sets it to 0 in StreamBuffer::init(). This makes the problem of unterminated string go away.

You might want to solve this in a cleaner way.

hinxx commented 3 months ago

This PR is obsolete ; DO NOT merge. Issue solved with https://github.com/paulscherrerinstitute/StreamDevice/issues/97#issuecomment-2004315393 and https://github.com/paulscherrerinstitute/StreamDevice/issues/97#issuecomment-2006325442.