Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reserveAdditionalCapacity with capacity=0 #93

Open
thavlik opened this issue Jun 26, 2019 · 3 comments
Open

reserveAdditionalCapacity with capacity=0 #93

thavlik opened this issue Jun 26, 2019 · 3 comments
Labels

Comments

@thavlik
Copy link

thavlik commented Jun 26, 2019

If capacity is zero, the while loop in FlatBuffersBuilder.reserveAdditionalCapacity will spin forever.

@mzaks mzaks added the bug label Jun 27, 2019
@thavlik
Copy link
Author

thavlik commented Jun 27, 2019

Additionally, specifying initialCapacity as anything other than 1 will lead to occasional alignment errors.

For embedding binary Data into a Swift flatbuffer, I used the following:

        try! builder.startVector(count: payload.count, elementSize: 1)
        var bytes = [UInt8]()
        bytes.append(contentsOf: payload)
        for val in bytes.lazy.reversed() {
            builder.insert(value: val)
        }
        let payloadOffset = builder.endVector()

I haven't investigated why the .lazy.reversed() call is necessary for it to be deserialized in-order by the Go library. Builder.CreateByteVector in builder.go doesn't do anything like that :)

This library is in disrepair, but I have gotten it to work - often through commenting lines out. Still, my overall verdict is that I deeply appreciate the work you've put into this library. Are pull requests welcome?

Thanks

@mzaks
Copy link
Owner

mzaks commented Jun 28, 2019

Hi Thomas,

first of all thanks for opening up the issue.

The matter of capacity=0 is in fact a bug which is trivial, however I discovered that in Xcode 10.2.1 the unit tests which are involved with FileReader are failing (they work with Xcode 10.1 though). I would like to investigate this first and it could take some time.

I am however curious about the "occasional alignment errors". Could you please reproduce it in a unit test? That would be very helpful. PRs are appreciated btw.

Regarding your question: "why .lazy.reversed()". It is due to the fact that the buffer is build backwards. The values are prepended to the buffer this is why you need to go in reverse. AFAIK all implementations of FlatBuffers build up the buffer backwards, because it simplifies the calculation of relative offsets. The only one implementation which does it forwards is flatcc.
I am also curious of, why are you using the Builder directly and not the generated code?

Last but not least and it might be the reason why you get misaligned or different results while using FlatBuffersSwift together with other FlatBuffers implementations. The default value for nullTerminatedUTF8 property in FlatBuffersBuilderOptions is set to be false. It would be safer to set it to true as it is actually standard in flatc (official FlatBuffers implementation). I set it to false because it is rather wasteful to append a null to every string taking in to a coin that every string is prepended with its length anyways. But in some languages (e.g. C++) it is simpler/efficient to convert a buffer to a string if it is zero / null terminated.

@thavlik
Copy link
Author

thavlik commented Jul 4, 2019

I am also curious of, why are you using the Builder directly and not the generated code?
One of my fields is a byte array. I didn't see a method to insert a Data, and I wasn't 100% sure if the utf-8 string encoding would produce desired behavior.

nullTerminatedUTF8 has always been false in my tests. So far I have no issues deserializing my buffers on servers written in both Go and JS.

There are "fixes" that I made to FlatbuffersSwiftCodegen to get it working in my environment. I will have to go through at some point and apply those changes as individual commits. For example, I had to remove the root table requirement entirely. There is also an issue with single-line comments in enum bodies causing some tables to be omitted from codegen. A function in one of the generated classes passes a Bool? to an init method that takes a non-optional Bool defaulting to false, which produces a compiler error that is fixed by append ?? false. I haven't had the time document these individually, but hopefully I can get back on it within the next couple weeks.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants