dvidelabs / flatcc

FlatBuffers Compiler and Library in C for C
Apache License 2.0
646 stars 182 forks source link

Misleading usage of a crippled function in the documentation examples #166

Closed byron-hawkins closed 3 years ago

byron-hawkins commented 3 years ago

The examples on the page https://github.com/dvidelabs/flatcc#building-a-buffer use the function flatcc_builder_get_direct_buffer, which is inherently crippled in that it quietly returns zero when the buffer is "too large". In my opinion, this function should simply be removed because it is a productivity hazard. If not, the function should at least print a wall-of-text error to stderr when it fails, instead of quietly returning zero. In any case, the examples should NEVER use flatcc_builder_get_direct_buffer. All examples throughout the documentation should use a working function such as flatcc_builder_finalize_buffer or flatc_builder_finalize_aligned_buffer.

mikkelfj commented 3 years ago

I agree that most examples should use the finalization functions suggested and that direct buffer should be a special case example with a notable warning. Would you be willing to add a contribution in this regard?

However, the runtime should not depend on and therefore should not print an error if it fails, especially due to a multitude of embedded use cases.

The direct buffer is very useful for many real world use cases where you know that the buffer is small, and where you through defines can increase the page size to have a large enough buffer. This can be significant in performance and memory sensitive situations, both of which are likely for flatcc deployments.

mikkelfj commented 3 years ago

I would also like to add that the finalization functions with dynamically allocated memory can also fail, especially on microcontrollers. In fact, they might be more likely to fail. Hence, it is always the applications responsibility to check for allocation results, and get direct buffer is no different in this regard.