From 83778ce56735e1f44ab4681b47470e904ef9fc9d Mon Sep 17 00:00:00 2001 From: Björn Linse Date: Wed, 29 Sep 2021 16:25:48 +0200 Subject: refactor(api): remove duplicated handle mpack encode/decode functions --- src/nvim/msgpack_rpc/helpers.c | 90 +++++++++++------------------------------- src/nvim/msgpack_rpc/helpers.h | 1 + 2 files changed, 24 insertions(+), 67 deletions(-) (limited to 'src') diff --git a/src/nvim/msgpack_rpc/helpers.c b/src/nvim/msgpack_rpc/helpers.c index 549016e751..2cb9edbc2b 100644 --- a/src/nvim/msgpack_rpc/helpers.c +++ b/src/nvim/msgpack_rpc/helpers.c @@ -22,42 +22,6 @@ static msgpack_zone zone; 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) \ - 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) { \ - return false; \ - } \ - \ - msgpack_object data; \ - msgpack_unpack_return ret = msgpack_unpack(obj->via.ext.ptr, \ - obj->via.ext.size, \ - NULL, \ - &zone, \ - &data); \ - \ - if (ret != MSGPACK_UNPACK_SUCCESS) { \ - return false; \ - } \ - \ - *arg = (handle_T)data.via.i64; \ - return true; \ - } \ - \ - 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); \ - msgpack_pack_int64(&pac, (handle_T)o); \ - 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); \ - } void msgpack_rpc_helpers_init(void) { @@ -65,10 +29,6 @@ void msgpack_rpc_helpers_init(void) msgpack_sbuffer_init(&sbuffer); } -HANDLE_TYPE_CONVERSION_IMPL(Buffer, buffer) -HANDLE_TYPE_CONVERSION_IMPL(Window, window) -HANDLE_TYPE_CONVERSION_IMPL(Tabpage, tabpage) - typedef struct { const msgpack_object *mobj; Object *aobj; @@ -226,28 +186,17 @@ case type: { \ break; } case MSGPACK_OBJECT_EXT: - 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); - break; - case kObjectTypeWindow: - cur.aobj->type = kObjectTypeWindow; - ret = msgpack_rpc_to_window(cur.mobj, &cur.aobj->data.integer); - break; - case kObjectTypeTabpage: - cur.aobj->type = kObjectTypeTabpage; - 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: - case kObjectTypeLuaRef: - break; + if (0 <= cur.mobj->via.ext.type && cur.mobj->via.ext.type <= EXT_OBJECT_TYPE_MAX) { + cur.aobj->type = (ObjectType)(cur.mobj->via.ext.type + EXT_OBJECT_TYPE_SHIFT); + msgpack_object data; + msgpack_unpack_return status = msgpack_unpack(cur.mobj->via.ext.ptr, cur.mobj->via.ext.size, NULL, &zone, &data); + + if (status != MSGPACK_UNPACK_SUCCESS || data.type != MSGPACK_OBJECT_POSITIVE_INTEGER) { + ret = false; + break; + } + cur.aobj->data.integer = (handle_T)data.via.i64; + ret = true; } break; #undef STR_CASE @@ -349,6 +298,17 @@ void msgpack_rpc_from_string(const String result, msgpack_packer *res) } } +static void msgpack_rpc_from_handle(ObjectType type, Integer o, msgpack_packer *res) + FUNC_ATTR_NONNULL_ARG(3) +{ + msgpack_packer pac; + msgpack_packer_init(&pac, &sbuffer, msgpack_sbuffer_write); + msgpack_pack_int64(&pac, (handle_T)o); + msgpack_pack_ext(res, sbuffer.size, (int8_t)type - EXT_OBJECT_TYPE_SHIFT); + msgpack_pack_ext_body(res, sbuffer.data, sbuffer.size); + msgpack_sbuffer_clear(&sbuffer); +} + typedef struct { const Object *aobj; bool container; @@ -393,13 +353,9 @@ void msgpack_rpc_from_object(const Object result, msgpack_packer *const res) msgpack_rpc_from_string(cur.aobj->data.string, res); break; case kObjectTypeBuffer: - msgpack_rpc_from_buffer(cur.aobj->data.integer, res); - break; case kObjectTypeWindow: - msgpack_rpc_from_window(cur.aobj->data.integer, res); - break; case kObjectTypeTabpage: - msgpack_rpc_from_tabpage(cur.aobj->data.integer, res); + msgpack_rpc_from_handle(cur.aobj->type, cur.aobj->data.integer, res); break; case kObjectTypeArray: { const size_t size = cur.aobj->data.array.size; diff --git a/src/nvim/msgpack_rpc/helpers.h b/src/nvim/msgpack_rpc/helpers.h index 0e4cd1be6d..ef3e851a6a 100644 --- a/src/nvim/msgpack_rpc/helpers.h +++ b/src/nvim/msgpack_rpc/helpers.h @@ -15,6 +15,7 @@ /// buffer/window/tabpage block inside ObjectType enum. This block yet cannot be /// split or reordered. #define EXT_OBJECT_TYPE_SHIFT kObjectTypeBuffer +#define EXT_OBJECT_TYPE_MAX (kObjectTypeTabpage - EXT_OBJECT_TYPE_SHIFT) #ifdef INCLUDE_GENERATED_DECLARATIONS # include "msgpack_rpc/helpers.h.generated.h" -- cgit From 413bb6b7a400f2c97ee43a6455d2120d1e18aece Mon Sep 17 00:00:00 2001 From: Björn Linse Date: Wed, 29 Sep 2021 16:56:25 +0200 Subject: refactor(api): cleanup modify_keymap and parse_keymap_opts --- src/nvim/api/private/helpers.c | 56 ++++++++++-------------------------------- src/nvim/msgpack_rpc/helpers.c | 5 ++-- 2 files changed, 16 insertions(+), 45 deletions(-) (limited to 'src') diff --git a/src/nvim/api/private/helpers.c b/src/nvim/api/private/helpers.c index 46ef41cc38..d03a61d1cc 100644 --- a/src/nvim/api/private/helpers.c +++ b/src/nvim/api/private/helpers.c @@ -818,13 +818,6 @@ Array string_to_array(const String input, bool crlf) void modify_keymap(Buffer buffer, bool is_unmap, String mode, String lhs, String rhs, Dict(keymap) *opts, Error *err) { - char *err_msg = NULL; // the error message to report, if any - char *err_arg = NULL; // argument for the error message format string - ErrorType err_type = kErrorTypeNone; - - char_u *lhs_buf = NULL; - char_u *rhs_buf = NULL; - bool global = (buffer == -1); if (global) { buffer = 0; @@ -858,17 +851,13 @@ void modify_keymap(Buffer buffer, bool is_unmap, String mode, String lhs, String CPO_TO_CPO_FLAGS, &parsed_args); if (parsed_args.lhs_len > MAXMAPLEN) { - err_msg = "LHS exceeds maximum map length: %s"; - err_arg = lhs.data; - err_type = kErrorTypeValidation; - goto fail_with_message; + api_set_error(err, kErrorTypeValidation, "LHS exceeds maximum map length: %s", lhs.data); + goto fail_and_free; } if (mode.size > 1) { - err_msg = "Shortname is too long: %s"; - err_arg = mode.data; - err_type = kErrorTypeValidation; - goto fail_with_message; + api_set_error(err, kErrorTypeValidation, "Shortname is too long: %s", mode.data); + goto fail_and_free; } int mode_val; // integer value of the mapping mode, to be passed to do_map() char_u *p = (char_u *)((mode.size) ? mode.data : "m"); @@ -880,18 +869,14 @@ void modify_keymap(Buffer buffer, bool is_unmap, String mode, String lhs, String && mode.size > 0) { // get_map_mode() treats unrecognized mode shortnames as ":map". // This is an error unless the given shortname was empty string "". - err_msg = "Invalid mode shortname: \"%s\""; - err_arg = (char *)p; - err_type = kErrorTypeValidation; - goto fail_with_message; + api_set_error(err, kErrorTypeValidation, "Invalid mode shortname: \"%s\"", (char *)p); + goto fail_and_free; } } if (parsed_args.lhs_len == 0) { - err_msg = "Invalid (empty) LHS"; - err_arg = ""; - err_type = kErrorTypeValidation; - goto fail_with_message; + api_set_error(err, kErrorTypeValidation, "Invalid (empty) LHS"); + goto fail_and_free; } bool is_noremap = parsed_args.noremap; @@ -904,16 +889,13 @@ void modify_keymap(Buffer buffer, bool is_unmap, String mode, String lhs, String // the given RHS was nonempty and not a , but was parsed as if it // were empty? assert(false && "Failed to parse nonempty RHS!"); - err_msg = "Parsing of nonempty RHS failed: %s"; - err_arg = rhs.data; - err_type = kErrorTypeException; - goto fail_with_message; + api_set_error(err, kErrorTypeValidation, "Parsing of nonempty RHS failed: %s", rhs.data); + goto fail_and_free; } } else if (is_unmap && parsed_args.rhs_len) { - err_msg = "Gave nonempty RHS in unmap command: %s"; - err_arg = (char *)parsed_args.rhs; - err_type = kErrorTypeValidation; - goto fail_with_message; + api_set_error(err, kErrorTypeValidation, + "Gave nonempty RHS in unmap command: %s", parsed_args.rhs); + goto fail_and_free; } // buf_do_map() reads noremap/unmap as its own argument. @@ -942,19 +924,7 @@ void modify_keymap(Buffer buffer, bool is_unmap, String mode, String lhs, String goto fail_and_free; } // switch - xfree(lhs_buf); - xfree(rhs_buf); - xfree(parsed_args.rhs); - xfree(parsed_args.orig_rhs); - - return; - -fail_with_message: - api_set_error(err, err_type, err_msg, err_arg); - fail_and_free: - xfree(lhs_buf); - xfree(rhs_buf); xfree(parsed_args.rhs); xfree(parsed_args.orig_rhs); return; diff --git a/src/nvim/msgpack_rpc/helpers.c b/src/nvim/msgpack_rpc/helpers.c index 2cb9edbc2b..f805858904 100644 --- a/src/nvim/msgpack_rpc/helpers.c +++ b/src/nvim/msgpack_rpc/helpers.c @@ -189,7 +189,8 @@ case type: { \ if (0 <= cur.mobj->via.ext.type && cur.mobj->via.ext.type <= EXT_OBJECT_TYPE_MAX) { cur.aobj->type = (ObjectType)(cur.mobj->via.ext.type + EXT_OBJECT_TYPE_SHIFT); msgpack_object data; - msgpack_unpack_return status = msgpack_unpack(cur.mobj->via.ext.ptr, cur.mobj->via.ext.size, NULL, &zone, &data); + msgpack_unpack_return status = msgpack_unpack(cur.mobj->via.ext.ptr, cur.mobj->via.ext.size, + NULL, &zone, &data); if (status != MSGPACK_UNPACK_SUCCESS || data.type != MSGPACK_OBJECT_POSITIVE_INTEGER) { ret = false; @@ -304,7 +305,7 @@ static void msgpack_rpc_from_handle(ObjectType type, Integer o, msgpack_packer * msgpack_packer pac; msgpack_packer_init(&pac, &sbuffer, msgpack_sbuffer_write); msgpack_pack_int64(&pac, (handle_T)o); - msgpack_pack_ext(res, sbuffer.size, (int8_t)type - EXT_OBJECT_TYPE_SHIFT); + msgpack_pack_ext(res, sbuffer.size, (int8_t)(type - EXT_OBJECT_TYPE_SHIFT)); msgpack_pack_ext_body(res, sbuffer.data, sbuffer.size); msgpack_sbuffer_clear(&sbuffer); } -- cgit