Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 3 commits into from

Conversation

hinxx
Copy link

@hinxx hinxx commented Mar 7, 2024

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

Hinko Kocevar added 2 commits March 7, 2024 15:59
if the string is exactly 64 bytes long it does not contain \0 and the scanString() includes the first byte of len member.
@hinxx
Copy link
Author

hinxx commented Mar 7, 2024

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
Copy link
Author

hinxx commented Mar 20, 2024

This PR is obsolete ; DO NOT merge.
Issue solved with #97 (comment) and #97 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants