1592 bug of mqttbufferreader #1593
Conversation
| var byte1 = _buffer[_position++]; | ||
| var byte2 = _buffer[_position++]; | ||
| var byte3 = _buffer[_position++]; | ||
| var byte0 = _buffer[_offset + Position]; |
There was a problem hiding this comment.
FYI: for "modern targets" this could be done via BinaryPrimitives.ReadUInt32BigEndian-method.
There was a problem hiding this comment.
I adopted this but enforced the reverse order. With the method you mentioned it depends on the CPU.
There was a problem hiding this comment.
Ah, right that "cleans up" the order automatically. MemoryMarshal.ReadXYZ would be the correct one.
The gain is one memory read instead of the individual bytes (which wil most likely be in the same cacheline). Effect will be only 1 bound-check instead of the 4.
|
|
||
| return result; | ||
| if (value <= 16383) |
There was a problem hiding this comment.
Can this be expressed as 0x7F, 0x3F_FF, 0x1F_FF_FF? To make it a bit more obvious where these constants come from? The old algorithm was clearer on that regard.
Or as comment just a link to the spec that dictates that encoding.
There was a problem hiding this comment.
Also when I read
correct and as I would assume only the most significant bit is relevant.With the constants used it's 1, 2, 3 bits that are "reserved for continuation", while only 1 is needed?
So the constants should be
0x7F, 0x7F_FF, 0x7F_FF_FF? So bigger ranges can be encoded with less bytes used. Or I'm missing something...
There was a problem hiding this comment.
I imported some documentation from the RFC to explain the values.
I do not really unterstand what you are referring to in the ReaderExtensions.
There was a problem hiding this comment.
I will merge this PR so that I can release a hotfix version soon. We can discuss this in another branch.
There was a problem hiding this comment.
I do not really unterstand what you are referring to in the ReaderExtensions.
Sorry, I mixed the plain and the encoding threshoulds.
No description provided.