Deprecated documentation mpack_cancel

[xor-gate]

I think mpack_cancel should now be mpack_discard: https://github.com/ludocode/mpack/blob/1aa7097bef92ef56c0c68ce4166ecfed14036865/src/mpack/mpack-reader.h#L345 https://github.com/ludocode/mpack/blob/1aa7097bef92ef56c0c68ce4166ecfed14036865/src/mpack/mpack-expect.h

[ludocode]

Ah, not quite. The documentation is wrong here but it's not meant to be mpack_discard(); it's actually meant to be mpack_reader_destroy_cancel().

The reason "cancel" is mentioned here is because it's necessary if you don't call one of the related "done" functions. If you call mpack_read_tag() and you get, say, a map, you have to either call mpack_done_map() at some point before calling mpack_reader_destroy(), or you have to call mpack_reader_destroy_cancel(). Otherwise if you try to destroy the reader and it's not in an error state, it will assert.

I'm actually in the process of getting rid of the cancel functions (it's on my painfully long TODO list.) The idea was that if you realize the data is corrupt or useless half way through and you want to stop reading it, you should be able to just close the reader. But the reader will assert because you haven't closed everything; it actually insists that you read the whole message. Cancel was a way around that. But I think instead, I'll just require that users flag an error manually before destroying the reader if they want to stop reading half way through. The (currently mostly unused) mpack_error_data will probably serve this purpose.

Anyway thanks for pointing out the incorrect documentation! I'm glad people are actually reading it :) I've got a handful of warnings coming out of doxygen when I build the docs, and I'll be turning up the warning levels if possible before a 1.0 release to help find more bugs like this.

[xor-gate]

Okay thanks for looking into this and explaining. And yes it is far easier to "click-trough" the doxygen documentation then the editor itself, so i'm using it often. Only serialize/deserialize key-values is very "raw" in mpack currently as there are no "kv" helpers and had to write my own. It would be nice to see something like mpack_write_kv(mpack_writer_t, const char *, type ). As you look into C11 you can "wrap" multiple writer function into one generic with the new _Generic selector.

We currently have our own RPC serialize API wrapped like this, as you can see we need to have overloaded functions for C++ because _Generic is not supported (it will result in the same code). Also I wrapped mpack_writer_cstr because it crashes when giving NULL pointer. Which we expect should serialize into Msgpack/Json null:

void di_rpc_serialize_cstr(mpack_writer_t *writer, const char *str);
void di_rpc_serialize_msg(mpack_writer_t *writer, struct di_rpc_msg *msg);
void di_rpc_serialize_error(mpack_writer_t *writer, struct di_rpc_error *error);
void di_rpc_serialize_info(mpack_writer_t *writer, struct di_rpc_info *info);

void di_rpc_serialize_result_i8(mpack_writer_t *writer,  struct di_rpc_msg *msg,  int8_t  value);
void di_rpc_serialize_result_i16(mpack_writer_t *writer, struct di_rpc_msg *msg,  int16_t value);
void di_rpc_serialize_result_i32(mpack_writer_t *writer, struct di_rpc_msg *msg,  int32_t value);
void di_rpc_serialize_result_i64(mpack_writer_t *writer, struct di_rpc_msg *msg,  int64_t value);
void di_rpc_serialize_result_u8(mpack_writer_t *writer,  struct di_rpc_msg *msg, uint8_t  value);
void di_rpc_serialize_result_u16(mpack_writer_t *writer, struct di_rpc_msg *msg, uint16_t value);
void di_rpc_serialize_result_u32(mpack_writer_t *writer, struct di_rpc_msg *msg, uint32_t value);
void di_rpc_serialize_result_u64(mpack_writer_t *writer, struct di_rpc_msg *msg, uint64_t value);
void di_rpc_serialize_result_bool(mpack_writer_t *writer, struct di_rpc_msg *msg, bool value);
void di_rpc_serialize_result_gps(mpack_writer_t *writer, struct di_rpc_msg *msg, struct di_rpc_info *info, struct di_rpc_sensor_data_gps *gps);

#ifndef __cplusplus
#define di_rpc_serialize(writer, structure)                 \
    _Generic((structure),                               \
              int8_t: mpack_write_i8,               \
             int16_t: mpack_write_i16,              \
             int32_t: mpack_write_i32,              \
             int64_t: mpack_write_i64,              \
             uint8_t: mpack_write_u8,               \
            uint16_t: mpack_write_u16,              \
            uint32_t: mpack_write_u32,              \
            uint64_t: mpack_write_u64,              \
                bool: mpack_write_bool,             \
        const char *: di_rpc_serialize_cstr,        \
                 struct di_rpc_msg  *: di_rpc_serialize_msg,   \
               struct di_rpc_error  *: di_rpc_serialize_error, \
                 struct di_rpc_info *: di_rpc_serialize_info   \
    )(writer, structure)

#define di_rpc_serialize_result(writer, msg, result)       \
    _Generic((result),                                 \
              int8_t: di_rpc_serialize_result_i8,  \
             int16_t: di_rpc_serialize_result_i16, \
             int32_t: di_rpc_serialize_result_i32, \
             int64_t: di_rpc_serialize_result_i64, \
             uint8_t: di_rpc_serialize_result_u8,  \
            uint16_t: di_rpc_serialize_result_u16, \
            uint32_t: di_rpc_serialize_result_u32, \
            uint64_t: di_rpc_serialize_result_u64, \
                bool: di_rpc_serialize_result_bool \
    )(writer, msg, result)

#endif

#define di_rpc_serialize_kv(writer, key, value) { \
        mpack_write_cstr(writer, key);    \
        di_rpc_serialize(writer, value);  \
    }

#define di_rpc_serialize_kv_nil(writer, key) { \
        mpack_write_cstr(writer, key); \
        mpack_write_nil(writer);       \
    }

#ifdef __cplusplus
}
#endif

#ifdef __cplusplus
    static inline void di_rpc_serialize(mpack_writer_t *writer, struct di_rpc_msg *msg) { di_rpc_serialize_msg(writer, msg); }
    static inline void di_rpc_serialize(mpack_writer_t *writer, struct di_rpc_error *error) { di_rpc_serialize_error(writer, error); }
    static inline void di_rpc_serialize(mpack_writer_t *writer, struct di_rpc_info *info) { di_rpc_serialize_info(writer, info); }
#endif

[ludocode]

KV write helpers are a really good idea. I should implement some and use them in some of my projects that use MPack. But I don't necessarily want to go the route of generics. I've been avoiding C11 stuff for a couple of reasons.

For one thing it's not well supported by older compilers. For example Travis still builds with GCC 4.6 despite it not being supported anymore (which is why the -std=c11 unit tests are disabled on Travis.) _Generic in particular is only available since GCC 4.9, even though -std=c11 gives __STDC_VERSION__ == 201112L all the way back to GCC 4.7. This is really frustrating because you can't even test for C11 support directly; you have to check the compiler version. At least with Clang it was implemented back in 3.0, and we can do __has_feature(c_generic_selections). But there is little hope of Microsoft ever supporting these.

For another, it doesn't play well with C++. MPack is meant to be integrated into C, C++ and Objective-C codebases and I don't really want to have to duplicate a bunch of stuff (e.g. using _Generic for C11 as opposed to a series of overloaded functions for C++). There are other features in C11 that I would love to use, for example anonymous unions and anonymous structs would dramatically simplify accessing the contents of an mpack_node_data_t. But C++ will probably never support anonymous structs, so I unfortunately have to avoid it as well.

Anyway the *_destroy_cancel() functions are gone from develop. The docs will be updated with the next release.

[xor-gate]

Yeah I totally agree on your points, your use-case and roadmap is clear and people who want to use C11 features can wrap themselves.