liteserver / binn

Binary Serialization
Apache License 2.0
440 stars 58 forks source link

Blob size implementation always uses 4-byte block #30

Closed ghost closed 4 years ago

ghost commented 4 years ago

Hello,

In specification, blob size can be 1 or 4-byte block. However, as far as I understand your code, blob size always uses 4-byte block. (I know very basic C).

If that is true, I think your implementation should be updated, to support 1-byte block.

May I ask if you could check?

Thank you,

kroggen commented 4 years ago

Indeed, you are right. It is differing from the documentation.

I could fix it but it can cause a problem for some users. The updated version will be able to read binns created by the previous versions but the previous versions will not be able to read a blob created by the updated code (for blobs > 127 bytes).

I guess I will update it in a separate branch.

ghost commented 4 years ago

Thank you, I understand your concern.

I think it's better if the implementation follows specification. It might help if you use semantic versioning (https://semver.org), and your users can decide which version they should use.

It also helps, when implementations in other languages produce 1-byte sizes for blobs. In such cases, current C code can't handle it :-(

Thanks again for your hard work.

kroggen commented 4 years ago

OK, I fixed the problem and I added tags to the code.

It is the first time I did it so there were no messages. Next time I will add comments.

Thank you for your suggestions!

ghost commented 4 years ago

Thank you very much, it is working :-)