msgpack-rpc: Refactor initializer and dispatcher

Use Map(String, rpc_method_handler_fn) for storing/retrieving rpc method
handlers in msgpack_rpc_init and msgpack_rpc_dispatch.

Also refactor serialization/validation functions in the
msgpack_rpc.c/msgpack_rpc_helpers.c modules to accept the new STR and BIN types.

3 files changed, 32 insertions, 25 deletions

diff --git a/scripts/msgpack-gen.lua b/scripts/msgpack-gen.lua
index 7967aad232..f868a67568 100644
--- a/scripts/msgpack-gen.lua
+++ b/scripts/msgpack-gen.lua
@@ -245,22 +245,24 @@ output:write('\n};\n\n')
 
 -- Generate a function that initializes method names with handler functions
 output:write([[
-static Map(cstr_t, uint64_t) *rpc_method_ids = NULL;
+static Map(String, rpc_method_handler_fn) *methods = NULL;
 
 void msgpack_rpc_init(void)
 {
-  rpc_method_ids = map_new(cstr_t, uint64_t)();
+  methods = map_new(String, rpc_method_handler_fn)();
 
 ]])
 
--- Msgpack strings must be copied to a 0-terminated temporary buffer before
--- searching in the map, so we keep track of the maximum method name length in
--- order to create the smallest possible buffer for xstrlcpy
+-- Keep track of the maximum method name length in order to avoid walking
+-- strings longer than that when searching for a method handler
 local max_fname_len = 0
 for i = 1, #api.functions do
   local fn = api.functions[i]
-  output:write('  map_put(cstr_t, uint64_t)(rpc_method_ids, "'
-               ..fn.name..'", '..i..');\n')
+  output:write('  map_put(String, rpc_method_handler_fn)(methods, '..
+               '(String) {.data = "'..fn.name..'", '..
+               '.size = sizeof("'..fn.name..'") - 1}, handle_'..
+               fn.name..');\n')
+
   if #fn.name > max_fname_len then
     max_fname_len = #fn.name
   end
@@ -275,22 +277,24 @@ Object msgpack_rpc_dispatch(uint64_t channel_id,
 {
   msgpack_object method = req->via.array.ptr[2];
   uint64_t method_id = method.via.u64;
+  rpc_method_handler_fn handler = NULL;
 
-  if (method.type == MSGPACK_OBJECT_BIN) {
-    char method_name[]]..(max_fname_len + 1)..[[];
-    xstrlcpy(method_name, method.via.bin.ptr, min(method.via.bin.size, ]] ..(max_fname_len)..[[) + 1);
-    method_id = map_get(cstr_t, uint64_t)(rpc_method_ids, method_name);
-    if (!method_id) {
-      method_id = UINT64_MAX;
-    }
+  if (method.type == MSGPACK_OBJECT_BIN || method.type == MSGPACK_OBJECT_STR) {
+]])
+output:write('    handler = map_get(String, rpc_method_handler_fn)')
+output:write('(methods, (String){.data=(char *)method.via.bin.ptr,')
+output:write('.size=min(method.via.bin.size, '..max_fname_len..')});\n')
+output:write([[
+  } else if (method_id <= ]]..#api.functions..[[) {
+    handler = rpc_method_handlers[method_id];
   }
+
+  if (!handler) {
+    handler = handle_missing_method;
+  }
+
+  return handler(channel_id, req, error);
+}
 ]])
-output:write('\n  // method_id=0 is specially handled')
-output:write('\n  assert(method_id > 0);')
-output:write('\n');
-output:write('\n  rpc_method_handler_fn handler = (method_id <= '..#api.functions..') ?')
-output:write('\n    rpc_method_handlers[method_id] : handle_missing_method;')
-output:write('\n  return handler(channel_id, req, error);')
-output:write('\n}\n')
 
 output:close()
diff --git a/src/nvim/os/msgpack_rpc.c b/src/nvim/os/msgpack_rpc.c
index 38670e0bf8..cf32736e16 100644
--- a/src/nvim/os/msgpack_rpc.c
+++ b/src/nvim/os/msgpack_rpc.c
@@ -235,7 +235,8 @@ static char *msgpack_rpc_validate(uint64_t *response_id, msgpack_object *req)
   }
 
   if (req->via.array.ptr[2].type != MSGPACK_OBJECT_POSITIVE_INTEGER
-      && req->via.array.ptr[2].type != MSGPACK_OBJECT_BIN) {
+      && req->via.array.ptr[2].type != MSGPACK_OBJECT_BIN
+      && req->via.array.ptr[2].type != MSGPACK_OBJECT_STR) {
     return "Method must be a positive integer or a string";
   }
 
diff --git a/src/nvim/os/msgpack_rpc_helpers.c b/src/nvim/os/msgpack_rpc_helpers.c
index eb43c3b2a6..eaba3b9785 100644
--- a/src/nvim/os/msgpack_rpc_helpers.c
+++ b/src/nvim/os/msgpack_rpc_helpers.c
@@ -81,12 +81,13 @@ bool msgpack_rpc_to_float(msgpack_object *obj, Float *arg)
 
 bool msgpack_rpc_to_string(msgpack_object *obj, String *arg)
 {
-  if (obj->type != MSGPACK_OBJECT_BIN) {
+  if (obj->type == MSGPACK_OBJECT_BIN || obj->type == MSGPACK_OBJECT_STR) {
+    arg->data = xmemdupz(obj->via.bin.ptr, obj->via.bin.size);
+    arg->size = obj->via.bin.size;
+  } else {
     return false;
   }
 
-  arg->data = xmemdupz(obj->via.bin.ptr, obj->via.bin.size);
-  arg->size = obj->via.bin.size;
   return true;
 }
 
@@ -111,6 +112,7 @@ bool msgpack_rpc_to_object(msgpack_object *obj, Object *arg)
       return msgpack_rpc_to_float(obj, &arg->data.floating);
 
     case MSGPACK_OBJECT_BIN:
+    case MSGPACK_OBJECT_STR:
       arg->type = kObjectTypeString;
       return msgpack_rpc_to_string(obj, &arg->data.string);