Merge pull request #15839 from bfredl/encgrugg

refactor(api): some cleanup

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 <Nop>, 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;

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,18 @@ 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 +299,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 +354,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;

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"