aeron
aeron copied to clipboard
Erroneous next fragment term offset computation
This is mostly a nitpick, and I'm not sure my analysis below is correct, so please correct me if I'm wrong.
In FragmentAssembler#handleFragment
, the term offset of the next fragment is computed as
BitUtil.align(offset + length + HEADER_LENGTH, FRAME_ALIGNMENT)
offset
and length
are (most likely) provided by TermReader#read
so their values are frameOffset + HEADER_LENGTH
and frameLength - HEADER_LENGTH
respectively, and therefore offset + length
is equal to frameOffset + frameLength
. Aligning that value to the FRAME_ALIGNMENT
to obtain the offset of the next frame makes sense. But adding + HEADER_LENGTH
before doing the alignment doesn't.
It just so happens that HEADER_LENGTH
is equal to FRAME_ALIGNMENT
and so the operations x -> BitUtil.align(x, FRAME_ALIGNMENT)
and x -> x + HEADER_LENGTH
commute. But if HEADER_LENGTH
was not a multiple of FRAME_ALIGNMENT
then this code would break. So for cleanliness and to avoid a bad surprise if the transport protocol ever changes (unlikely I guess), it would be best to instead write BitUtil.align(offset + length, FRAME_ALIGNMENT) + HEADER_LENGTH
.