[Go] generator does not handle repeating groups where `blockLength > sum(fields)`

[sergiu128]

There are cases where a repeating group's block length is bigger than the sum of its fields. For example:

<group name="NoMDEntries" id="268" description="Number of entries in Market Data message" blockLength="32" dimensionType="groupSize">
    <field name="MDEntryPx" id="270" type="PRICENULL9" description="Market Data entry price" offset="0" semanticType="Price"/>
    <field name="MDEntrySize" id="271" type="Int32NULL" description="Market Data entry size" offset="8" semanticType="Qty"/>
    <field name="SecurityID" id="48" type="Int32" description="Security ID" offset="12" semanticType="int"/>
    <field name="RptSeq" id="83" type="uInt32" description="Market Data entry sequence number per instrument update" offset="16" semanticType="int"/>
    <field name="NumberOfOrders" id="346" type="Int32NULL" description="In Book entry - aggregate number of orders at given price level" offset="20" semanticType="int"/>
    <field name="MDPriceLevel" id="1023" type="uInt8" description="Aggregate book level" offset="24" semanticType="int"/>
    <field name="MDUpdateAction" id="279" type="MDUpdateAction" description=" Market Data update action" offset="25" semanticType="int"/>
    <field name="MDEntryType" id="269" type="MDEntryTypeBook" description="Market Data entry type" offset="26" semanticType="char"/>
</group>

Here, blockLength is 32 but sum(fields) = 27 < 32. The Go generator should account for those 32 - 27 = 5 bytes of padding but it does not. The C++ generator does the right thing.

This is what the Go generator does right now:

func Decode(...) {
    // ...
    if m.NoMDEntriesInActingVersion(actingVersion) {
	var NoMDEntriesBlockLength uint16
	if err := _m.ReadUint16(_r, &NoMDEntriesBlockLength); err != nil {
	    return err
	}
	var NoMDEntriesNumInGroup uint8
	if err := _m.ReadUint8(_r, &NoMDEntriesNumInGroup); err != nil {
	    return err
	}
	if cap(m.NoMDEntries) < int(NoMDEntriesNumInGroup) {
	    m.NoMDEntries = make([]MDIncrementalRefreshBook46NoMDEntries, NoMDEntriesNumInGroup)
	}
	m.NoMDEntries = m.NoMDEntries[:NoMDEntriesNumInGroup]
	for i := range m.NoMDEntries {
	    if err := m.NoMDEntries[i].Decode(_m, _r, actingVersion, uint(NoMDEntriesBlockLength)); err != nil {
		    return err
	    }
	}
	io.CopyN(ioutil.Discard, _r, 5)
    }
    // ...
}

This is what the Go generator should do:

func Decode(...) {
    // ...
    if m.NoMDEntriesInActingVersion(actingVersion) {
	var NoMDEntriesBlockLength uint16
	if err := _m.ReadUint16(_r, &NoMDEntriesBlockLength); err != nil {
	    return err
	}
	var NoMDEntriesNumInGroup uint8
	if err := _m.ReadUint8(_r, &NoMDEntriesNumInGroup); err != nil {
	    return err
	}
	if cap(m.NoMDEntries) < int(NoMDEntriesNumInGroup) {
	    m.NoMDEntries = make([]MDIncrementalRefreshBook46NoMDEntries, NoMDEntriesNumInGroup)
	}
	m.NoMDEntries = m.NoMDEntries[:NoMDEntriesNumInGroup]
	for i := range m.NoMDEntries {
	    if err := m.NoMDEntries[i].Decode(_m, _r, actingVersion, uint(NoMDEntriesBlockLength)); err != nil {
		    return err
	    }
	    io.CopyN(ioutil.Discard, _r, 5)
	}
    }
    // ...
}

This is what the C++ generator does:

inline NoMDEntries &next()
{
    if (m_index >= m_count)
    {
        throw std::runtime_error("index >= count [E108]");
    }
    m_offset = *m_positionPtr;
    if (SBE_BOUNDS_CHECK_EXPECT(((m_offset + m_blockLength) > m_bufferLength), false))
    {
        throw std::runtime_error("buffer too short for next group index [E108]");
    }
    *m_positionPtr = m_offset + m_blockLength;
    ++m_index;

    return *this;
}

I have a potential fix here, however I'm unsure of its correctness, I need some input from you.

You can run the example above here.

CC @theodosiosandreou

[vyazelenko]

/cc @ethanf