Merge PR #1121 'Full compliance with msgpack-RPC'

.ci/api-python.sh

@@ -23,7 +23,7 @@ fi
 
 valgrind_check "$tmpdir"
 
-export NEOVIM_SPAWN_ARGV="[\"valgrind\", \"-q\", \"--track-origins=yes\", \"--leak-check=yes\", \"--suppressions=$suppressions\", \"--log-file=$tmpdir/valgrind-%p.log\", \"../build/bin/nvim\", \"-u\", \"NONE\", \"--embedded-mode\"]"
+export NVIM_SPAWN_ARGV="[\"valgrind\", \"-q\", \"--track-origins=yes\", \"--leak-check=yes\", \"--suppressions=$suppressions\", \"--log-file=$tmpdir/valgrind-%p.log\", \"../build/bin/nvim\", \"-u\", \"NONE\", \"--embedded-mode\"]"
 if ! nosetests --verbosity=2 --nologcapture; then
 	valgrind_check "$tmpdir"
 	exit 1

scripts/msgpack-gen.lua

@@ -91,6 +91,7 @@ output:write([[
 #include <assert.h>
 #include <msgpack.h>
 
+#include "nvim/map.h"
 #include "nvim/log.h"
 #include "nvim/os/msgpack_rpc.h"
 #include "nvim/os/msgpack_rpc_helpers.h"
@@ -241,12 +242,49 @@ for i = 1, #api.functions do
 end
 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;
+
+void msgpack_rpc_init(void)
+{
+  rpc_method_ids = map_new(cstr_t, uint64_t)();
+
+]])
+
+-- 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
+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')
+  if #fn.name > max_fname_len then
+    max_fname_len = #fn.name
+  end
+end
+
+output:write('\n}\n\n')
+
+output:write([[
+#define min(X, Y) (X < Y ? X : Y)
+
 Object msgpack_rpc_dispatch(uint64_t channel_id,
-                            uint64_t method_id,
                             msgpack_object *req,
                             Error *error)
 {
+  msgpack_object method = req->via.array.ptr[2];
+  uint64_t method_id = method.via.u64;
+
+  if (method.type == MSGPACK_OBJECT_RAW) {
+    char method_name[]]..(max_fname_len + 1)..[[];
+    xstrlcpy(method_name, method.via.raw.ptr, min(method.via.raw.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;
+    }
+  }
 ]])
 output:write('\n  // method_id=0 is specially handled')
 output:write('\n  assert(method_id > 0);')

src/nvim/eval.c

@@ -6493,8 +6493,8 @@ static struct fst {
   {"searchpair",      3, 7, f_searchpair},
   {"searchpairpos",   3, 7, f_searchpairpos},
   {"searchpos",       1, 4, f_searchpos},
-  {"send_call",       3, 3, f_send_call},
+  {"send_call",       2, 64, f_send_call},
-  {"send_event",      3, 3, f_send_event},
+  {"send_event",      2, 64, f_send_event},
   {"setbufvar",       3, 3, f_setbufvar},
   {"setcmdpos",       1, 1, f_setcmdpos},
   {"setline",         2, 2, f_setline},
@@ -12632,13 +12632,19 @@ static void f_send_call(typval_T *argvars, typval_T *rettv)
     return;
   }
 
+  Array args = ARRAY_DICT_INIT;
+
+  for (typval_T *tv = argvars + 2; tv->v_type != VAR_UNKNOWN; tv++) {
+    ADD(args, vim_to_object(tv));
+  }
+
   bool errored;
   Object result;
   if (!channel_send_call((uint64_t)argvars[0].vval.v_number,
-                          (char *)argvars[1].vval.v_string,
+                         (char *)argvars[1].vval.v_string,
-                          vim_to_object(&argvars[2]),
+                         args,
-                          &result,
+                         &result,
-                          &errored)) {
+                         &errored)) {
     EMSG2(_(e_invarg2), "Channel doesn't exist");
     return;
   }
@@ -12673,9 +12679,15 @@ static void f_send_event(typval_T *argvars, typval_T *rettv)
     return;
   }
 
+  Array args = ARRAY_DICT_INIT;
+
+  for (typval_T *tv = argvars + 2; tv->v_type != VAR_UNKNOWN; tv++) {
+    ADD(args, vim_to_object(tv));
+  }
+
   if (!channel_send_event((uint64_t)argvars[0].vval.v_number,
                           (char *)argvars[1].vval.v_string,
-                          vim_to_object(&argvars[2]))) {
+                          args)) {
     EMSG2(_(e_invarg2), "Channel doesn't exist");
     return;
   }
@@ -19149,7 +19161,9 @@ static void apply_job_autocmds(Job *job, char *name, char *type, char *str)
 
 static void script_host_eval(char *method, typval_T *argvars, typval_T *rettv)
 {
-  Object result = provider_call(method, vim_to_object(argvars));
+  Array args = ARRAY_DICT_INIT;
+  ADD(args, vim_to_object(argvars));
+  Object result = provider_call(method, args);
 
   if (result.type == kObjectTypeNil) {
     return;

src/nvim/ex_cmds2.c

@@ -3251,8 +3251,9 @@ static void script_host_execute(char *method, exarg_T *eap)
   char *script = (char *)script_get(eap, eap->arg);
 
   if (!eap->skip) {
-    String str = cstr_to_string(script ? script : (char *)eap->arg);
+    Array args = ARRAY_DICT_INIT;
-    Object result = provider_call(method, STRING_OBJ(str));
+    ADD(args, STRING_OBJ(cstr_to_string(script ? script : (char *)eap->arg)));
+    Object result = provider_call(method, args);
     // We don't care about the result, so free it just in case a bad provider
     // returned something
     msgpack_rpc_free_object(result);
@@ -3266,18 +3267,19 @@ static void script_host_execute_file(char *method, exarg_T *eap)
   char buffer[MAXPATHL];
   vim_FullName(eap->arg, (uint8_t *)buffer, sizeof(buffer), false);
 
-  String file = cstr_to_string(buffer);
+  Array args = ARRAY_DICT_INIT;
-  Object result = provider_call(method, STRING_OBJ(file));
+  ADD(args, STRING_OBJ(cstr_to_string(buffer)));
+  Object result = provider_call(method, args);
   msgpack_rpc_free_object(result);
 }
 
 static void script_host_do_range(char *method, exarg_T *eap)
 {
-  Array arg = {0, 0, 0};
+  Array args = ARRAY_DICT_INIT;
-  ADD(arg, INTEGER_OBJ(eap->line1));
+  ADD(args, INTEGER_OBJ(eap->line1));
-  ADD(arg, INTEGER_OBJ(eap->line2));
+  ADD(args, INTEGER_OBJ(eap->line2));
-  ADD(arg, STRING_OBJ(cstr_to_string((char *)eap->arg)));
+  ADD(args, STRING_OBJ(cstr_to_string((char *)eap->arg)));
-  Object result = provider_call(method, ARRAY_OBJ(arg));
+  Object result = provider_call(method, args);
   msgpack_rpc_free_object(result);
 }
 

src/nvim/ops.c

@@ -5232,7 +5232,8 @@ static void get_clipboard(int name)
 
   struct yankreg *reg = &y_regs[CLIP_REGISTER];
   free_register(reg);
-  Object result = provider_call("clipboard_get", NIL);
+  Array args = ARRAY_DICT_INIT;
+  Object result = provider_call("clipboard_get", args);
 
   if (result.type != kObjectTypeArray) {
     goto err;
@@ -5278,12 +5279,15 @@ static void set_clipboard(int name)
     copy_register(reg, &y_regs[0]);
   }
 
-  Array lines = {0, 0, 0};
+  Array lines = ARRAY_DICT_INIT;
 
   for (int i = 0; i < reg->y_size; i++) {
     ADD(lines, STRING_OBJ(cstr_to_string((char *)reg->y_array[i])));
   }
 
-  Object result = provider_call("clipboard_set", ARRAY_OBJ(lines));
+  Array args = ARRAY_DICT_INIT;
+  ADD(args, ARRAY_OBJ(lines));
+
+  Object result = provider_call("clipboard_set", args);
   msgpack_rpc_free_object(result);
 }

src/nvim/os/channel.c

@@ -143,40 +143,48 @@ bool channel_exists(uint64_t id)
     && channel->enabled;
 }
 
-/// Sends event/data to channel
+/// Sends event/arguments to channel
 ///
 /// @param id The channel id. If 0, the event will be sent to all
 ///        channels that have subscribed to the event type
 /// @param name The event name, an arbitrary string
-/// @param arg The event arg
+/// @param args Array with event arguments
-/// @return True if the data was sent successfully, false otherwise.
+/// @return True if the event was sent successfully, false otherwise.
-bool channel_send_event(uint64_t id, char *name, Object arg)
+bool channel_send_event(uint64_t id, char *name, Array args)
 {
   Channel *channel = NULL;
 
   if (id > 0) {
     if (!(channel = pmap_get(uint64_t)(channels, id)) || !channel->enabled) {
-      msgpack_rpc_free_object(arg);
+      msgpack_rpc_free_array(args);
       return false;
     }
-    send_event(channel, name, arg);
+    send_event(channel, name, args);
   } else {
-    broadcast_event(name, arg);
+    broadcast_event(name, args);
   }
 
   return true;
 }
 
+/// Sends a method call to a channel
+///
+/// @param id The channel id
+/// @param name The method name, an arbitrary string
+/// @param args Array with method arguments
+/// @param[out] result Pointer to return value received from the channel
+/// @param[out] error True if the return value is an error
+/// @return True if the call was sent successfully, false otherwise.
 bool channel_send_call(uint64_t id,
                        char *name,
-                       Object arg,
+                       Array args,
                        Object *result,
                        bool *errored)
 {
   Channel *channel = NULL;
 
   if (!(channel = pmap_get(uint64_t)(channels, id)) || !channel->enabled) {
-    msgpack_rpc_free_object(arg);
+    msgpack_rpc_free_array(args);
     return false;
   }
 
@@ -190,13 +198,13 @@ bool channel_send_call(uint64_t id,
         "while processing a RPC call",
         channel->id);
     *result = STRING_OBJ(cstr_to_string(buf));
-    msgpack_rpc_free_object(arg);
+    msgpack_rpc_free_array(args);
     return false;
   }
 
   uint64_t request_id = channel->next_request_id++;
   // Send the msgpack-rpc request
-  send_request(channel, request_id, name, arg);
+  send_request(channel, request_id, name, args);
 
   EventSource channel_source = channel->is_job
     ? job_event_source(channel->data.job)
@@ -415,21 +423,21 @@ static void send_error(Channel *channel, uint64_t id, char *err)
 static void send_request(Channel *channel,
                          uint64_t id,
                          char *name,
-                         Object arg)
+                         Array args)
 {
   String method = {.size = strlen(name), .data = name};
-  channel_write(channel, serialize_request(id, method, arg, &out_buffer, 1));
+  channel_write(channel, serialize_request(id, method, args, &out_buffer, 1));
 }
 
 static void send_event(Channel *channel,
                        char *name,
-                       Object arg)
+                       Array args)
 {
   String method = {.size = strlen(name), .data = name};
-  channel_write(channel, serialize_request(0, method, arg, &out_buffer, 1));
+  channel_write(channel, serialize_request(0, method, args, &out_buffer, 1));
 }
 
-static void broadcast_event(char *name, Object arg)
+static void broadcast_event(char *name, Array args)
 {
   kvec_t(Channel *) subscribed;
   kv_init(subscribed);
@@ -442,14 +450,14 @@ static void broadcast_event(char *name, Object arg)
   });
 
   if (!kv_size(subscribed)) {
-    msgpack_rpc_free_object(arg);
+    msgpack_rpc_free_array(args);
     goto end;
   }
 
   String method = {.size = strlen(name), .data = name};
   WBuffer *buffer = serialize_request(0,
                                       method,
-                                      arg,
+                                      args,
                                       &out_buffer,
                                       kv_size(subscribed));
 

src/nvim/os/msgpack_rpc.c

@@ -39,15 +39,14 @@ WBuffer *msgpack_rpc_call(uint64_t channel_id,
     return serialize_response(response_id, err, NIL, sbuffer);
   }
 
-  uint64_t method_id = req->via.array.ptr[2].via.u64;
+  if (req->via.array.ptr[2].type == MSGPACK_OBJECT_POSITIVE_INTEGER
-
+     && req->via.array.ptr[2].via.u64 == 0) {
-  if (method_id == 0) {
     return serialize_metadata(response_id, channel_id, sbuffer);
   }
 
   // dispatch the call
   Error error = { .set = false };
-  Object rv = msgpack_rpc_dispatch(channel_id, method_id, req, &error);
+  Object rv = msgpack_rpc_dispatch(channel_id, req, &error);
   // send the response
   msgpack_packer response;
   msgpack_packer_init(&response, sbuffer, msgpack_sbuffer_write);
@@ -119,7 +118,7 @@ void msgpack_rpc_error(char *msg, msgpack_packer *res)
 /// Serializes a msgpack-rpc request or notification(id == 0)
 WBuffer *serialize_request(uint64_t request_id,
                            String method,
-                           Object arg,
+                           Array args,
                            msgpack_sbuffer *sbuffer,
                            size_t refcount)
   FUNC_ATTR_NONNULL_ARG(4)
@@ -135,12 +134,12 @@ WBuffer *serialize_request(uint64_t request_id,
 
   msgpack_pack_raw(&pac, method.size);
   msgpack_pack_raw_body(&pac, method.data, method.size);
-  msgpack_rpc_from_object(arg, &pac);
+  msgpack_rpc_from_array(args, &pac);
   WBuffer *rv = wstream_new_buffer(xmemdup(sbuffer->data, sbuffer->size),
                                    sbuffer->size,
                                    refcount,
                                    free);
-  msgpack_rpc_free_object(arg);
+  msgpack_rpc_free_array(args);
   msgpack_sbuffer_clear(sbuffer);
   return rv;
 }
@@ -235,8 +234,9 @@ static char *msgpack_rpc_validate(uint64_t *response_id, msgpack_object *req)
     return "Message type must be 0";
   }
 
-  if (req->via.array.ptr[2].type != MSGPACK_OBJECT_POSITIVE_INTEGER) {
+  if (req->via.array.ptr[2].type != MSGPACK_OBJECT_POSITIVE_INTEGER
-    return "Method id must be a positive integer";
+      && req->via.array.ptr[2].type != MSGPACK_OBJECT_RAW) {
+    return "Method must be a positive integer or a string";
   }
 
   if (req->via.array.ptr[3].type != MSGPACK_OBJECT_ARRAY) {

src/nvim/os/msgpack_rpc.h

@@ -21,6 +21,11 @@ typedef Object (*rpc_method_handler_fn)(uint64_t channel_id,
                                         msgpack_object *req,
                                         Error *error);
 
+
+/// Initializes the msgpack-rpc method table
+void msgpack_rpc_init(void);
+
+
 /// Dispatches to the actual API function after basic payload validation by
 /// `msgpack_rpc_call`. It is responsible for validating/converting arguments
 /// to C types, and converting the return value back to msgpack types.
@@ -33,11 +38,11 @@ typedef Object (*rpc_method_handler_fn)(uint64_t channel_id,
 /// @param error Pointer to error structure
 /// @return Some object
 Object msgpack_rpc_dispatch(uint64_t channel_id,
-                            uint64_t method_id,
                             msgpack_object *req,
                             Error *error)
   FUNC_ATTR_NONNULL_ARG(2) FUNC_ATTR_NONNULL_ARG(3);
 
+
 #ifdef INCLUDE_GENERATED_DECLARATIONS
 # include "os/msgpack_rpc.h.generated.h"
 #endif

src/nvim/os/provider.c

@@ -98,7 +98,7 @@ bool provider_register(char *method, uint64_t channel_id)
   return true;
 }
 
-Object provider_call(char *method, Object arg)
+Object provider_call(char *method, Array args)
 {
   uint64_t channel_id = get_provider_for(method);
 
@@ -109,13 +109,13 @@ Object provider_call(char *method, Object arg)
              "Provider for \"%s\" is not available",
              method);
     report_error(buf);
-    msgpack_rpc_free_object(arg);
+    msgpack_rpc_free_array(args);
     return NIL;
   }
 
   bool error = false;
   Object result = NIL;
-  channel_send_call(channel_id, method, arg, &result, &error);
+  channel_send_call(channel_id, method, args, &result, &error);
 
   if (error) {
     report_error(result.data.string.data);

src/nvim/os_unix.c

@@ -54,6 +54,7 @@
 #include "nvim/os/shell.h"
 #include "nvim/os/signal.h"
 #include "nvim/os/job.h"
+#include "nvim/os/msgpack_rpc.h"
 
 #if defined(HAVE_SYS_IOCTL_H)
 # include <sys/ioctl.h>
@@ -164,6 +165,7 @@ void mch_init(void)
   mac_conv_init();
 #endif
 
+  msgpack_rpc_init();
   event_init();
 }