logic_analyzer
logic_analyzer copied to clipboard
gillham
logic_analyzer_sigrok: Fix buffer underrun when dumping triggered capture
logicIndex is unsigned, so logicIndex < 0 was never true. Also, since we're dumping in reverse order, we need to to predecrement, not post-decrement
Buffer underrun caused dumping random corrupt garbage instead of captured data
Hello! This bug took lots of time to find despite its sheer simplicity, i hope you're interested in the fix.
Thanks for submitting this. I fixed a similar one in commit 739144d and meant to check the rest of the variables for a similar issue and didn't get there. I think I would rather just make it signed since the Mega buffer is 7KB at the most. Let me think about it as maybe I need to fix my other unsigned -> signed change if I want to resurrect the 'agla_megaram' branch which can get to 56KB buffer so would need it to be unsigned.
I don't think moving the logicIndex-- is doing the right thing as it appears the data at index zero will not be returned.
With the if check like that when it is zero you are resetting the logicIndex before we use the zero value.
I think the if (logicIndex == 0) change is the only part we really need right?
Nevermind, I guess it uses zero and the next time around the if is true. I will test this out locally this week hopefully.
Exactly. The sequence is: wrap, store, increment. "Wrap" means rolling over from 1024 to 0. So after the final loop iteration the index stays at (last_used + 1). 0 is not a problem at all; but after storing logic_buffer[1023] your counter will hold 1024. Pre-decrement does it all.
You may of course just change index to signed; it looked kinda ugly for me; but that's a matter of pure taste