Skip to content

Conversation

@moteus
Copy link
Contributor

@moteus moteus commented Jan 8, 2022

Problem that if buf has partial line e.g. SETB fi the mobdebug will not recognize it as a set breakpoint command.

Problem that if buf has partial line e.g. `SETB fi` the mobdebug will not recognize it as a set breakpoint command.
@pkulchenko
Copy link
Owner

Thank you for the patch; I agree there may be a problem with a partial buffer longer than 5 bytes, but do you have a test case or a description where this may happen?

@moteus
Copy link
Contributor Author

moteus commented Jan 8, 2022

do you have a test case or a description where this may happen?

No. I just read the code :)

@pkulchenko
Copy link
Owner

No. I just read the code :)

I sometimes do this too ;)

I wonder if this is the issue that may be causing pkulchenko/ZeroBraneStudio#909 (as that issue seems to be related to un/setting async breakpoints, which is what this fragment handles):

...it looks like it happens mostly when the Stack window is shown. So, the application is stopped, asetb command is triggered, but the stack is being retrieved first and the result of the stack command gets mixed with the asetb command.

@pkulchenko
Copy link
Owner

@moteus, I was re-reading the current logic and don't think it should cause this issue, as is_pending(server) check only reads 1 byte and only if the buf value is nil, so I don't see a scenario for it to get into having a partial buffer with 5 characters.

If you suspect that SETB fi may be accepted, then I think it shouldn't pass line 525 with the partial check. Yes, it seems like it may get into a situation when SETB fi partial is stored in the buffer, which will attempt readnext with a negative number (on line 521), but it should trigger a runtime error. Hm, I wonder if I can reproduce this and if this may cause some leftovers in the buffer that can "fuse" with the next command...

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