closes#9136
- Treat empty {rhs} like <Nop>
- getchar.c: Pull "repl. MapArg termcodes" into func
The "preprocessing code" surrounding the replace_termcodes calls needs
to invoke replace_termcodes, and also check if RHS is equal to "<Nop>".
To reduce code duplication, factor this out into a helper function.
Also add an rhs_is_noop flag to MapArguments; buf_do_map_explicit
expects an empty {rhs} string for "<Nop>", but also needs to distinguish
that from something like ":map lhs<cr>" where no {rhs} was provided.
- getchar.c: Use allocated buffer for rhs in MapArgs
Since the MAXMAPLEN limit does not apply to the RHS of a mapping (or
else an RHS that calls a really long autoload function from a plugin
would be incorrectly rejected as being too long), use an allocated
buffer for RHS rather than a static buffer of length MAXMAPLEN + 1.
- Mappings LHS and RHS can contain literal space characters, newlines, etc.
- getchar.c: replace_termcodes in str_to_mapargs
It makes sense to do this; str_to_mapargs is, intuitively, supposed to
take a "raw" command string and parse it into a totally "do_map-ready"
struct.
- api/vim.c: Update lhs, rhs len after replace_termcodes
Fixes a bug in which replace_termcodes changes the length of lhs or rhs,
but the later search through the mappings/abbreviations hashtables
still uses the old length value. This would cause the search to fail
erroneously and throw 'E31: No such mapping' errors or 'E24: No such
abbreviation' errors.
- getchar: Create new map_arguments struct
So that a string of map arguments can be parsed into a more useful, more
portable data structure.
- getchar.c: Add buf_do_map function
Exactly the same as the old do_map, but replace the hardcoded references
to the global `buf_T* curbuf` with a function parameter so that we can
invoke it from nvim_buf_set_keymap.
- Remove gettext calls in do_map error handling
Problem: when processing cycle such as
:for pat in [' \ze*', ' \zs*']
: try
: let l = matchlist('x x', pat)
: $put ='E888 NOT detected for ' . pat
: catch
: $put ='E888 detected for ' . pat
: endtry
:endfor
`:let l = …` throwing an error causes this error to be caught after
color_cmdline attempts to get callback for highlighting next line (the one with
`$put = 'E888 NOT…`). Saving/restoring state prevents this from happening.
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.
Since data.integer is a different (larger) integer type than
data.{buffer,window,tabpage}, we cannot abuse the union by using
data.integer to access the value for all 4 types. Instead, remove the
{buffer,window,tabpage} members and always use the integer member.
In order to accomodate this, perform distinct validation and coercion
between the Integer type and Buffer/Window/Tabpage types in
object_to_vim, msgpack_rpc helpers, and gendispatch.lua.
This ought to prevent stack overflow, but I do not see this actually working:
*lua* code crashes with stack overflow when trying to deserialize msgpack from
Neovim, Neovim is fine even if nesting level is increased 100x (though test
becomes very slow); not sure how recursive function may survive this. So it
looks like there are currently only two positive effects:
1. NULL lists are returned as empty (#4596).
2. Functional tests are slightly more fast. Very slightly. Checked for Release
build for test/functional/eval tests because benchmarking of debug mode is
not very useful.
What works:
1. ShaDa file dumping: header, registers, jump list, history, search patterns,
substitute strings, variables.
2. ShaDa file reading: registers, global marks, variables.
Most was not tested.
TODO:
1. Merging.
2. Reading history, local marks, jump and buffer lists.
3. Documentation update.
4. Converting some data from &encoding.
5. Safer variant of dumping viminfo (dump to temporary file then rename).
6. Removing old viminfo code (currently masked with `#if 0` in a ShaDa file for
reference).
- Add error type information to `Error`
- Rename `set_api_error` to `api_set_error` for consistency with other api_*
functions/macros.
- Refactor the api_set_error macro to accept formatted strings and error types
- Improve error messages
- Wrap error messages with gettext macro
- Refactor msgpack-rpc serialization to transform Error instances into [type,
message] arrays
- Add error type information to API metadata
- Normalize nvim->client and client->nvim error handling(change
channel_send_call to accept an Error pointer instead of the `errored` boolean
pointer)
- Use macro to initialize Error structures
This is how API dispatching worked before this commit:
- The generated `msgpack_rpc_dispatch` function receives a the `msgpack_packer`
argument.
- The response is incrementally built while validating/calling the API.
- Return values/errors are also packed into the `msgpack_packer` while the
final response is being calculated.
Now the `msgpack_packer` argument is no longer provided, and the
`msgpack_rpc_dispatch` function returns `Object`/`Error` values to
`msgpack_rpc_call`, which will use those values to build the response in a
single pass.
This was done because the new `channel_send_call` function created the
possibility of having recursive API invocations, and this wasn't possible when
sharing a single `msgpack_sbuffer` across call frames(it was shared implicitly
through the `msgpack_packer` instance).
Since we only start to build the response when the necessary information has
been computed, it's now safe to share a single `msgpack_sbuffer` instance
across all channels and API invocations.
Some other changes also had to be performed:
- Handling of the metadata discover was moved to `msgpack_rpc_call`
- Expose more types as subtypes of `Object`, this was required to forward the
return value from `msgpack_rpc_dispatch` to `msgpack_rpc_call`
- Added more helper macros for casting API types to `Object`
any
- The 'stripdecls.py' script replaces declarations in all headers by includes to
generated headers.
`ag '#\s*if(?!ndef NEOVIM_).*((?!#\s*endif).*\n)*#ifdef INCLUDE_GENERATED'`
was used for this.
- Add and integrate gendeclarations.lua into the build system to generate the
required includes.
- Add -Wno-unused-function
- Made a bunch of old-style definitions ANSI
This adds a requirement: all type and structure definitions must be present
before INCLUDE_GENERATED_DECLARATIONS-protected include.
Warning: mch_expandpath (path.h.generated.h) was moved manually. So far it is
the only exception.
Relates to issue #760
These coverity warnings are of the form:
>>> CID 62602: Buffer not null terminated (BUFFER_SIZE_WARNING)
>>> Calling strncpy with a maximum size argument of 256 bytes...
This is caused by strncpy not alway NULL-terminated the destination buffer
(for example in the case where strlen(src) >= size(dst)). It's better to
replace that with (x)strlcpy, which always NULL-terminates.
Most of these are related to the set_api_error macro, which uses strncpy.
The error struct is used (for example) in msgpack_rpc_error, where strlen is
executed on it, so it needs to be NULL-terminated. (x)strlcpy, unlike
strncpy, always NULL-terminates the destination buffer.
Relevant parts of the coverity report:
*** CID 62602: Buffer not null terminated (BUFFER_SIZE_WARNING)
/src/nvim/api/vim.c: 236 in vim_set_current_buffer()
230 if (try_end(err)) {
231 return;
232 }
233
234 char msg[256];
235 snprintf(msg, sizeof(msg),
"failed to switch to buffer %d", (int)buffer);
>>> CID 62602: Buffer not null terminated (BUFFER_SIZE_WARNING)
>>> Calling strncpy with a maximum size argument of 256 bytes on
>>> destination array "err->msg" of size 256 bytes might leave the
>>> destination string unterminated.
236 set_api_error(msg, err);
237 return;
238 }
239
240 try_end(err);
241 }
*** CID 62603: Buffer not null terminated (BUFFER_SIZE_WARNING)
/src/nvim/api/private/helpers.c: 70 in try_end()
64 } else if (msg_list != NULL && *msg_list != NULL) {
65 int should_free;
66 char *msg = (char *)get_exception_string(*msg_list,
67 ET_ERROR,
68 NULL,
69 &should_free);
>>> CID 62603: Buffer not null terminated (BUFFER_SIZE_WARNING)
>>> Calling strncpy with a maximum size argument of 256 bytes on
>>> destination array "err->msg" of size 256 bytes might leave the
>>> destination string unterminated.
70 strncpy(err->msg, msg, sizeof(err->msg));
71 err->set = true;
72 free_global_msglist();
73
74 if (should_free) {
75 free(msg);
/src/nvim/api/private/helpers.c: 78 in try_end()
72 free_global_msglist();
73
74 if (should_free) {
75 free(msg);
76 }
77 } else if (did_throw) {
>>> CID 62603: Buffer not null terminated (BUFFER_SIZE_WARNING)
>>> Calling strncpy with a maximum size argument of 256 bytes on
>>> destination array "err->msg" of size 256 bytes might leave the
>>> destination string unterminated.
78 set_api_error((char *)current_exception->value, err);
79 }
80
81 return err->set;
82 }
83
*** CID 62604: Buffer not null terminated (BUFFER_SIZE_WARNING)
/src/nvim/api/private/helpers.c: 592 in set_option_value_err()
586 opt_flags)))
587 {
588 if (try_end(err)) {
589 return;
590 }
591
>>> CID 62604: Buffer not null terminated (BUFFER_SIZE_WARNING)
>>> Calling strncpy with a maximum size argument of 256 bytes on
>>> destination array "err->msg" of size 256 bytes might leave the
>>> destination string unterminated.
592 set_api_error(errmsg, err);
593 }
*** CID 62605: Buffer not null terminated (BUFFER_SIZE_WARNING)
/src/nvim/os/server.c: 114 in server_start()
108 if (addr_len > sizeof(ip) - 1) {
109 // Maximum length of a ip address buffer is 15(eg: 255.255.255.255)
110 addr_len = sizeof(ip);
111 }
112
113 // Extract the address part
>>> CID 62605: Buffer not null terminated (BUFFER_SIZE_WARNING)
>>> Calling strncpy with a maximum size argument of 16 bytes on
>>> destination array "ip" of size 16 bytes might leave the destination
>>> string unterminated.
114 strncpy(ip, addr, addr_len);
115
116 int port = NEOVIM_DEFAULT_TCP_PORT;
117
118 if (*ip_end == ':') {
119 char *port_end;
/src/nvim/os/server.c: 88 in server_start()
82
83 void server_start(char *endpoint, ChannelProtocol prot)
84 {
85 char addr[ADDRESS_MAX_SIZE];
86
87 // Trim to `ADDRESS_MAX_SIZE`
>>> CID 62605: Buffer not null terminated (BUFFER_SIZE_WARNING)
>>> Calling strncpy with a maximum size argument of 256 bytes on
>>> destination array "addr" of size 256 bytes might leave the
>>> destination string unterminated.
88 strncpy(addr, endpoint, sizeof(addr));
89
90 // Check if the server already exists
91 if (map_has(cstr_t)(servers, addr)) {
92 EMSG2("Already listening on %s", addr);
93 return;
*** CID 62606: Buffer not null terminated (BUFFER_SIZE_WARNING)
/src/nvim/os/server.c: 186 in server_stop()
180 void server_stop(char *endpoint)
181 {
182 Server *server;
183 char addr[ADDRESS_MAX_SIZE];
184
185 // Trim to `ADDRESS_MAX_SIZE`
>>> CID 62606: Buffer not null terminated (BUFFER_SIZE_WARNING)
>>> Calling strncpy with a maximum size argument of 256 bytes on
>>> destination array "addr" of size 256 bytes might leave the
>>> destination string unterminated.
187
188 if ((server = map_get(cstr_t)(servers, addr)) == NULL) {
189 EMSG2("Not listening on %s", addr);
190 return;
191 }
