[tarantool-patches] Re: [PATCH v7 2/7] box: introduce OPT_ARRAY opt_type to decode arrays

[Kirill Shcherbatov]

> While the previous callback is self-explanatory, this one requires
> a comment. Actually a comment for both won't hurt. What should the
> callback return? who does own the memory used by the return value? 
> What is the error handling convention?

+/**
+ * Decode enum stored in MsgPack.
+ * @param str encoded data pointer (next to MsgPack ENUM header).
+ * @param len str length.
+ * @retval string index or hmax if the string is not found.
+ */
 typedef int64_t (*opt_def_to_enum_cb)(const char *str, uint32_t len);
+
+/**
+ * Decode MsgPack array callback.
+ * All memory allocations returned by opt_def_to_array_cb with opt
+ * [out] argument should be managed manually.
+ * @param str encoded data pointer (next to MsgPack ARRAY header).
+ * @param len array length (items count).
+ * @param [out] opt pointer to store resulting value.
+ * @retval 0 on success.
+ * @retval -1 on error.
+ */
 typedef int (*opt_def_to_array_cb)(const char **str, uint32_t len, char *opt);
 > The comment became worse, not better.
- /** If not NULL, used to get a value by a string. */
+ /** MsgPack data decode callbacks. */


> Otherwise LGTM.