astitcher commented on pull request #295: URL: https://github.com/apache/qpid-proton/pull/295#issuecomment-776848512
> It’s more and more difficult for me to understand what confuses you. That's funny, because I feel similarly about your comments! I guess we're talking somewhat at cross purposes. > we're talking about `typedef uint32_t pn_sequence_t;`. > while it will work as written if `lwm = 0xffffffff` and `hwm = 0x3` then `lwm> hwm`. If you are talking about the mathematics of unsigned ints then this is trivially true! However in the terms of the underlying buffer it is not true. The buffer is designed so that both hwm and lwm only ever increase and so eventually they will both wrap around from MAX_UINT to 0. This is fine and is exactly how unsigned ints are supposed to work. However it is an invariant of the buffer that lwm refers to a later buffer entry than hwm even when wrapped. > if you are talking about the inaccuracy of the initial check, then explain it in more detail. What inaccuracy? So I suppose that answer here is no - That isn't my concern. > because `hwm-lwm <0x80000000` is generally dangerous in my opinion. I don't understand this at all! And I think it's actually wrong! What this test does is effectively check the most significant bit of the result of the subtraction. Since we've subtracted unsigned ints the answer can never be really negative, but the answer will still have the most significant bit set if the answer would have been negative if the result was signed. So as long as the difference between hwm and lwm is always less than MAX_UINT/2 (which is the case here) the test I'm suggesting makes sure to run the loop until lwm equals hwm even if hwm is wrapped around and lwm is not. Whereas the one you are suggesting does not work in this case. Actually the current test does in practice work correctly too but only because it ends up testing lwm==hwm so I think it's an accident given the way the code is written. And if for some reason the code was changed so that lwm was incremented by more than 1 it would no longer be correct. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
