From ba2f615cd40d5d809d1a141c7b098e3bd22ff7bb Mon Sep 17 00:00:00 2001 From: ZyX Date: Sat, 16 Jul 2016 02:26:04 +0300 Subject: functests: Test for error conditions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit During testing found the following bugs: 1. msgpack-gen.lua script is completely unprepared for Float values either in return type or in arguments. Specifically: 1. At the time of writing relevant code FLOAT_OBJ did not exist as well as FLOATING_OBJ, but it would be used by msgpack-gen.lua should return type be Float. I added FLOATING_OBJ macros later because did not know that msgpack-gen.lua uses these _OBJ macros, otherwise it would be FLOAT_OBJ. 2. msgpack-gen.lua should use .data.floating in place of .data.float. But it did not expect that .data subattribute may have name different from lowercased type name. 2. vim_replace_termcodes returned its argument as-is if it receives an empty string (as well as _vim_id*() functions did). But if something in returned argument lives in an allocated memory such action will cause double free: once when freeing arguments, then when freeing return value. It did not cause problems yet because msgpack bindings return empty string as {NULL, 0} and nothing was actually allocated. 3. New code in msgpack-gen.lua popped arguments in reversed order, making lua bindings’ signatures be different from API ones. --- src/nvim/msgpack_rpc/helpers.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/nvim/msgpack_rpc/helpers.c') diff --git a/src/nvim/msgpack_rpc/helpers.c b/src/nvim/msgpack_rpc/helpers.c index 5137b375f0..64a018f5c3 100644 --- a/src/nvim/msgpack_rpc/helpers.c +++ b/src/nvim/msgpack_rpc/helpers.c @@ -117,7 +117,7 @@ bool msgpack_rpc_to_object(const msgpack_object *const obj, Object *const arg) case MSGPACK_OBJECT_FLOAT: { STATIC_ASSERT(sizeof(Float) == sizeof(cur.mobj->via.f64), "Msgpack floating-point size does not match API integer"); - *cur.aobj = FLOATING_OBJ(cur.mobj->via.f64); + *cur.aobj = FLOAT_OBJ(cur.mobj->via.f64); break; } #define STR_CASE(type, attr, obj, dest, conv) \ -- cgit From ca4c8b7f8a7b9543ef01157cb5ab94783f624ac6 Mon Sep 17 00:00:00 2001 From: ZyX Date: Sun, 22 Jan 2017 04:55:26 +0300 Subject: api: Allow kObjectTypeNil to be zero without breaking compatibility --- src/nvim/msgpack_rpc/helpers.c | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) (limited to 'src/nvim/msgpack_rpc/helpers.c') diff --git a/src/nvim/msgpack_rpc/helpers.c b/src/nvim/msgpack_rpc/helpers.c index 64a018f5c3..972d5b3441 100644 --- a/src/nvim/msgpack_rpc/helpers.c +++ b/src/nvim/msgpack_rpc/helpers.c @@ -20,13 +20,20 @@ static msgpack_zone zone; static msgpack_sbuffer sbuffer; +/// Value by which objects represented as EXT type are shifted +/// +/// Subtracted when packing, added when unpacking. Used to allow moving +/// buffer/window/tabpage block inside ObjectType enum. This block yet cannot be +/// split or reordered. +#define EXT_OBJECT_TYPE_SHIFT kObjectTypeBuffer + #define HANDLE_TYPE_CONVERSION_IMPL(t, lt) \ bool msgpack_rpc_to_##lt(const msgpack_object *const obj, \ Integer *const arg) \ FUNC_ATTR_NONNULL_ALL \ { \ if (obj->type != MSGPACK_OBJECT_EXT \ - || obj->via.ext.type != kObjectType##t) { \ + || obj->via.ext.type + EXT_OBJECT_TYPE_SHIFT != kObjectType##t) { \ return false; \ } \ \ @@ -51,7 +58,8 @@ static msgpack_sbuffer sbuffer; msgpack_packer pac; \ msgpack_packer_init(&pac, &sbuffer, msgpack_sbuffer_write); \ msgpack_pack_int64(&pac, (handle_T)o); \ - msgpack_pack_ext(res, sbuffer.size, kObjectType##t); \ + msgpack_pack_ext(res, sbuffer.size, \ + kObjectType##t - EXT_OBJECT_TYPE_SHIFT); \ msgpack_pack_ext_body(res, sbuffer.data, sbuffer.size); \ msgpack_sbuffer_clear(&sbuffer); \ } @@ -211,7 +219,7 @@ bool msgpack_rpc_to_object(const msgpack_object *const obj, Object *const arg) break; } case MSGPACK_OBJECT_EXT: { - switch (cur.mobj->via.ext.type) { + switch ((ObjectType)(cur.mobj->via.ext.type + EXT_OBJECT_TYPE_SHIFT)) { case kObjectTypeBuffer: { cur.aobj->type = kObjectTypeBuffer; ret = msgpack_rpc_to_buffer(cur.mobj, &cur.aobj->data.integer); @@ -227,6 +235,15 @@ bool msgpack_rpc_to_object(const msgpack_object *const obj, Object *const arg) ret = msgpack_rpc_to_tabpage(cur.mobj, &cur.aobj->data.integer); break; } + case kObjectTypeNil: + case kObjectTypeBoolean: + case kObjectTypeInteger: + case kObjectTypeFloat: + case kObjectTypeString: + case kObjectTypeArray: + case kObjectTypeDictionary: { + break; + } } break; } @@ -350,6 +367,9 @@ void msgpack_rpc_from_object(const Object result, msgpack_packer *const res) kv_push(stack, ((APIToMPObjectStackItem) { &result, false, 0 })); while (kv_size(stack)) { APIToMPObjectStackItem cur = kv_last(stack); + STATIC_ASSERT(kObjectTypeWindow == kObjectTypeBuffer + 1 + && kObjectTypeTabpage == kObjectTypeWindow + 1, + "Buffer, window and tabpage enum items are in order"); switch (cur.aobj->type) { case kObjectTypeNil: { msgpack_pack_nil(res); -- cgit From 22d3ce9c29c163470dbb652199acbb7c82e3dca1 Mon Sep 17 00:00:00 2001 From: ZyX Date: Sun, 22 Jan 2017 05:35:48 +0300 Subject: msgpack_rpc: Fix #HANDLE_TYPE_CONVERSION_IMPL Function declarations generator is able to handle properly only the *first* function definition that is in macros, and only if it is the first entity in the macros. So msgpack_rpc_from_* was already really a static function, additionally its attributes were useless. This commit switches to explicit declarations and makes generated functions static. --- src/nvim/msgpack_rpc/helpers.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) (limited to 'src/nvim/msgpack_rpc/helpers.c') diff --git a/src/nvim/msgpack_rpc/helpers.c b/src/nvim/msgpack_rpc/helpers.c index 972d5b3441..94a4f5ce3e 100644 --- a/src/nvim/msgpack_rpc/helpers.c +++ b/src/nvim/msgpack_rpc/helpers.c @@ -28,9 +28,11 @@ static msgpack_sbuffer sbuffer; #define EXT_OBJECT_TYPE_SHIFT kObjectTypeBuffer #define HANDLE_TYPE_CONVERSION_IMPL(t, lt) \ - bool msgpack_rpc_to_##lt(const msgpack_object *const obj, \ - Integer *const arg) \ - FUNC_ATTR_NONNULL_ALL \ + static bool msgpack_rpc_to_##lt(const msgpack_object *const obj, \ + Integer *const arg) \ + REAL_FATTR_NONNULL_ALL REAL_FATTR_WARN_UNUSED_RESULT; \ + static bool msgpack_rpc_to_##lt(const msgpack_object *const obj, \ + Integer *const arg) \ { \ if (obj->type != MSGPACK_OBJECT_EXT \ || obj->via.ext.type + EXT_OBJECT_TYPE_SHIFT != kObjectType##t) { \ @@ -52,8 +54,9 @@ static msgpack_sbuffer sbuffer; return true; \ } \ \ - void msgpack_rpc_from_##lt(Integer o, msgpack_packer *res) \ - FUNC_ATTR_NONNULL_ARG(2) \ + static void msgpack_rpc_from_##lt(Integer o, msgpack_packer *res) \ + REAL_FATTR_NONNULL_ARG(2); \ + static void msgpack_rpc_from_##lt(Integer o, msgpack_packer *res) \ { \ msgpack_packer pac; \ msgpack_packer_init(&pac, &sbuffer, msgpack_sbuffer_write); \ -- cgit From ae4adcc70735a89bffb110bcf9d5a993b0786c4d Mon Sep 17 00:00:00 2001 From: ZyX Date: Sat, 28 Jan 2017 03:47:15 +0300 Subject: gendeclarations: Make declarations generator work with macros funcs Now it checks functions also after every semicolon and closing figure brace, possibly preceded by whitespaces (tabs and spaces). This should make messing with declarations in macros not needed. --- src/nvim/msgpack_rpc/helpers.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'src/nvim/msgpack_rpc/helpers.c') diff --git a/src/nvim/msgpack_rpc/helpers.c b/src/nvim/msgpack_rpc/helpers.c index 94a4f5ce3e..ec35d56978 100644 --- a/src/nvim/msgpack_rpc/helpers.c +++ b/src/nvim/msgpack_rpc/helpers.c @@ -30,9 +30,7 @@ static msgpack_sbuffer sbuffer; #define HANDLE_TYPE_CONVERSION_IMPL(t, lt) \ static bool msgpack_rpc_to_##lt(const msgpack_object *const obj, \ Integer *const arg) \ - REAL_FATTR_NONNULL_ALL REAL_FATTR_WARN_UNUSED_RESULT; \ - static bool msgpack_rpc_to_##lt(const msgpack_object *const obj, \ - Integer *const arg) \ + FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT \ { \ if (obj->type != MSGPACK_OBJECT_EXT \ || obj->via.ext.type + EXT_OBJECT_TYPE_SHIFT != kObjectType##t) { \ @@ -55,8 +53,7 @@ static msgpack_sbuffer sbuffer; } \ \ static void msgpack_rpc_from_##lt(Integer o, msgpack_packer *res) \ - REAL_FATTR_NONNULL_ARG(2); \ - static void msgpack_rpc_from_##lt(Integer o, msgpack_packer *res) \ + FUNC_ATTR_NONNULL_ARG(2) \ { \ msgpack_packer pac; \ msgpack_packer_init(&pac, &sbuffer, msgpack_sbuffer_write); \ -- cgit From 62fde319360e27a86c0deba0053ff230f80ca772 Mon Sep 17 00:00:00 2001 From: ZyX Date: Sun, 29 Jan 2017 01:29:51 +0300 Subject: api: Also shift numbers in api_metadata output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes problem introduced by “api: Allow kObjectTypeNil to be zero without breaking compatibility”: apparently there are clients which use metadata and there are which aren’t. For the first that commit would not be needed, for the second that commit misses this critical piece. --- src/nvim/msgpack_rpc/helpers.c | 7 ------- 1 file changed, 7 deletions(-) (limited to 'src/nvim/msgpack_rpc/helpers.c') diff --git a/src/nvim/msgpack_rpc/helpers.c b/src/nvim/msgpack_rpc/helpers.c index ec35d56978..21acd6a394 100644 --- a/src/nvim/msgpack_rpc/helpers.c +++ b/src/nvim/msgpack_rpc/helpers.c @@ -20,13 +20,6 @@ static msgpack_zone zone; static msgpack_sbuffer sbuffer; -/// Value by which objects represented as EXT type are shifted -/// -/// Subtracted when packing, added when unpacking. Used to allow moving -/// buffer/window/tabpage block inside ObjectType enum. This block yet cannot be -/// split or reordered. -#define EXT_OBJECT_TYPE_SHIFT kObjectTypeBuffer - #define HANDLE_TYPE_CONVERSION_IMPL(t, lt) \ static bool msgpack_rpc_to_##lt(const msgpack_object *const obj, \ Integer *const arg) \ -- cgit