Repository navigation
Fix closing submessages with known size - #166
Conversation
joto
left a comment
joto
left a comment
There was a problem hiding this comment.
The code was already quite complicated with the special case of the m_rollback_pos = size_is_known, now the different meanings of where m_pos is also dependent on whether this is a fixed or variable sized submessage.
The clean solution would probably be to have a std::variant keeping those two cases apart (plus the case where we are not in a submessage at all), but that would mean moving to C+17.
close_submessage() returned early for submessages opened with a known size without resetting m_pos in the parent, so any further write to the parent hit the "existing submessage" assert. This affected e.g. packed_field_fixed32 with a size argument, as shown in the tutorial. Reset m_pos when closing such a submessage.
1bced71 to
a15ab8f
Compare
|
Fair; I simplified this so that we are now just simply resetting |
|
I also decided against some kind of std::variant-like version for this fix because it would increase the memory size. |
When a submessage was opened with a known size (e.g.
packed_field_fixed32 field{pw, 1, 2}, as in the tutorial), closing it never reset the parent'sm_pos. Any later write to the parent then hit the "existing submessage" assert in debug builds.Closing now resets the parent
and asserts that exactly. Tests cover the correct,sizebytes were writtentoo-few and too-many element cases, plus a plain known-size submessage.