search

ludocode / mpack

MPack - A C encoder/decoder for the MessagePack serialization format / msgpack.org[C]
MIT License
533 stars 82 forks source link

Implementation for expect_ext_* functions #61

Closed volks73 closed 6 years ago

volks73 commented 6 years ago

This pull request should fix #59 and includes implementations for the expect_ext_* functions. I basically copied and modified the expect_bin_* functions for the implementation, which I figured would be the easiest way to contribute the missing functions. Documentation is added for the new functions.

I have included tests for the new functions similar to the expect_bin_* function tests, but I could not get one of them to work. I marked the problematic test with some comments.

Hopefully this will be of use.

ludocode commented 6 years ago

Nice, looks good!

I was originally planning on having the expect ext functions match a specific type. It seems unlikely that you'd expect to receive ext data without also expecting it to be of a specific type. We should probably call those functions something else though (like mpack_expect_ext_type()), and I agree it makes sense to have the basic mpack_expect_ext() expect just any ext and return everything about it. So we can merge these now and then add type checker helpers later.

I'd like to spend a bit of time debugging that failing test (hopefully tomorrow). Returning a wrong error code seems pretty harmless though so in the meantime I'm good to merge this anyway. Thanks!

volks73 commented 6 years ago

Thank you for the merge!

I was originally planning on having the expect ext functions match a specific type.

Ha, I did not even think about this when I was implementing the expect ext functions, but now that you mention it, it would be cool to have separate mpack_expect_ext_type functions.

ludocode commented 6 years ago

I just fixed a couple issues on develop. The most important one that might affect you is that now if any error occurs, the type is always set to zero. I'm trying to stick to an overall strategy of setting/returning zero everywhere on error so that there's no risk of using uninitialized data.

volks73 commented 6 years ago

Thanks for the heads up.

I'm trying to stick to an overall strategy of setting/returning zero everywhere on error so that there's no risk of using uninitialized data.

I like this strategy too. It reduces "quirks" in the API and adds predictability.