OleksandrKvl / sbepp

C++ implementation of the FIX Simple Binary Encoding
https://oleksandrkvl.github.io/sbepp/
MIT License
39 stars 4 forks source link

Variable length arrays must be forbidden in the composite #26

Closed ujos closed 4 months ago

ujos commented 4 months ago

Based on answer in the https://github.com/real-logic/simple-binary-encoding/issues/485 it seems variable-length arrays cannot be used inside the composites:

        <composite name="varStrEncoding">
            <type name="length" primitiveType="uint32"/>
            <type name="varData" primitiveType="char" length="0"/>
        </composite>

        <composite name="TestComposite">
            <ref name="someField" type="varStrEncoding"/>
        </composite>

If that is true, then SBE C++ Compiler must return an error if one tries to use s composite with the zero length array within another composite.

OleksandrKvl commented 4 months ago

Yes, it doesn't make any sense to have them. But read here:

sbeppc is not a reference SBE schema validation tool. It implements the standard only to extent required for correct data representation. It does its best but please don't rely on it for complete schema validation.

For example, the above varData field is represented by sbeppc as a static_array_ref of size 0. Why? Because it doesn't matter, this type in XML is used only to specify length type for a data field. Just another bad design choice in SBE. The main purpose of sbeppc is to handle already correct schemas, not to detect all possible mistakes.

OleksandrKvl commented 4 months ago

To be more precise, it's not that I'm against more safety checks in general but taking into account my time budget and the absence of strong real-world need or requests from the actual clients for them, it's just not worth it.

ujos commented 4 months ago

I just say that I've spend couple of hours to figure out why I cannot set value to the var-length string field inside the composite. And at the end as I've figured I've created wrong schema. I think would be great to save someone from this kind of problems.

ujos commented 4 months ago

Of course that was my fault. But, that is your choice, do you want to save your users from stupid errors or not. The less painful the learning process, the higher chance you will get more users for your library (if that is what you want of course).

OleksandrKvl commented 4 months ago

The problem is wider here, it's very hard (if possible at all) to have a complete XML schema validation. Especially taking into account poor SBE specification. That's why from the beginning it was not the goal and schema compiler was written without extensive checks in mind beyond those that are required to generate C++ code. So I see no reason to fix a single problem mentioned in this issue, it's not the complete solution and the chance that anyone else will face the same issue is minuscule.