../src/nvim/event/rstream.c:119:44: warning: format specifies type 'void *' but the argument has type 'Stream *' (aka 'struct stream *') [-Wformat-pedantic]
DLOG("Closing Stream (%p): %s (%s)", stream,
~~ ^~~~~~
../src/nvim/event/stream.c:95:30: warning: format specifies type 'void *' but the argument has type 'Stream *' (aka 'struct stream *') [-Wformat-pedantic]
DLOG("closing Stream: %p", stream);
~~ ^~~~~~
../src/nvim/msgpack_rpc/channel.c:71:72: warning: format specifies type 'void *' but the argument has type 'Stream *' (aka 'struct stream *') [-Wformat-pedantic]
DLOG("rpc ch %" PRIu64 " in-stream=%p out-stream=%p", channel->id, in, out);
~~ ^~
../src/nvim/msgpack_rpc/channel.c:71:76: warning: format specifies type 'void *' but the argument has type 'Stream *' (aka 'struct stream *') [-Wformat-pedantic]
DLOG("rpc ch %" PRIu64 " in-stream=%p out-stream=%p", channel->id, in, out);
~~ ^~~
../src/nvim/msgpack_rpc/channel.c:226:28: warning: format specifies type 'void *' but the argument has type 'Stream *' (aka 'struct stream *') [-Wformat-pedantic]
channel->id, count, stream);
^~~~~~
According to [MessagePack RPC specification](https://github.com/msgpack-rpc/msgpack-rpc),
message ID must be 32-bit unsigned integer. But Neovim implementation
uses uint64_t instead of uint32_t. This can have wrong results in the
case of large ids or a malformed request, for example:
Actual response: [1,18446744073709551615,[1,"Message is not an array"],null]
Expected response: [1,4294967295,[1,"Message is not an array"],null]
The issue does not affect RPC clients written in dynamically-typed
languages like Python. Wrong type of sequence id number breaks RPC
clients written statically typed languages like C/C++/Golang: all of
them expect uint32_t as message id.
Examples:
11268ba2be/src/msgpack/rpc/protocol.h (L27)https://github.com/ugorji/go/blob/master/codec/msgpack.go#L993closes#8850
Using a sentinel value in the response-id is ambiguous because the
msgpack-rpc spec allows all values (including zero/max). And clients
control the id, so we can't be sure they won't use the sentinel value.
Instead of a sentinel value, check the message type explicitly.
ref #8850
Note about shada.c:
- shada_read_next_item_start was intentionally shadowing `unpacked` and
`i` because many of the macros (e.g. ADDITIONAL_KEY) implicitly
depended on those variable names.
- Macros were changed to parameterize `unpacked` (but not `i`). Macros
like CLEAR_GA_AND_ERROR_OUT do control-flow (goto), so any other
approach is messy.
Give embeders a chance to set up nvim, by processing a request before
startup. This allows an external UI to show messages and prompts from
--cmd and buffer loading (e.g. swap files)
channel.c: WIP remove redundant method check and added FUNC_ATTR_NONNULL_ALL macro
channel.c channel_defs.h helpers.c: added Error field to RequestEvent, added no_op handler func
channel.c: use const char* instead of string and cleanup
channel.c; channel_defs.h; helpers.c: removed error from event again; send errors directly to the channel without using handlers and events
channel.c: fixed memory leak and lint errors
api/private/dispatch.c; api/vim.c; msgpack_rpc/channel.c msgpack_rpc/helpers.c added Error* field to msgpack_get_handler_for; further refactored channel.c
channel.c:323 changed order of evaluation in if statement
channel.c: removed superflous whitespace
dispatch.c: review comment
When compiling with CMAKE_BUILD_TYPE=RelWithDebInfo, several
-Wmaybe-uninitialized warnings are printed. These were thought to
be false positives (#5061); there are no control paths that lead
to an uninitialized value. However, when gcc is run in -O2 mode,
it makes a mistake while generating the necessary logic.
Specifically, for the code:
...
int = 0; // Index of the server whose address equals addr.
for (; i < watchers.ga_len; i++) {
watcher = ((SocketWatcher **)watchers.ga_data)[i];
// <snip>
}
if (i >= watchers.ga_len) {
ELOG("Not listening on %s", addr);
return;
}
...
Gcc generates:
...
<+98>: cmp %ebx, %ebp
<+100>: jg 0x530f13 <server_stop+55>
<+102>: cmp %ebp, ebx
<+104>: jl 0x530f7e <server_stop+162>
...
Normally, the if statement should catch the only control path
where watcher is not assigned: watchers.ga_len <= 0. When
compiled, the assembly lines 98 and 100 correspond to checking if
i < watchers.ga_len, and the lines 102 and 104 correspond to
checking if i >= watchers.ga_len. The assembly seems to compare
ebp (which is watchers.ga_len) with ebx (which is i), and jump
if greater; then do the same comparison and jump if less. This is
where gcc makes a mistake: it flips the order of the cmp
instruction. This means that the REAL behavior is first check if
i < watchers.ga_len and then check if i < watchers.ga_len. Which
means the code inside the if statement is NEVER executed; no
combination of i and watchers.ga_len will ever trigger ELOG().
So not only is this a use of an uninitialized value if
watchers.ga_len == 0 (or technically, if it's less than zero too),
it also clobbers any error detection if the for loop reaches the
last entry (which would normally cause i == watchers.ga_len too).
This commit fixes this issue by adding a bool to keep track of
whether a watcher was found during the loop. This makes gcc
generate the correct code, avoiding both bugs.
With the old behavior, if a GUI makes a blocking request that requires user
interaction (like nvim_input()), it would not get any screen updates.
The client, not nvim, should decide how to handle notifications during a
pending request. If an rplugin wants to avoid async calls while a sync call is
busy, it likely wants to avoid processing async calls while another async call
also is handled as well.
This may break the expectation of some existing rplugins. For compatibility,
remote/define.vim reimplements the old behavior. Clients can opt-out by
specifying `sync=urgent`.
- Legacy hosts should be updated to use `sync=urgent`. They could add a flag
indicating which async methods are always safe to call and which must wait
until the main loop returns.
- New hosts can expose the full asyncness, they don't need to offer both
behaviors.
ref #6532
ref #1398d83868fe90
- Establish ERROR log level as "critical". Such errors are rare and will
be valuable when users encounter unusual circumstances.
- Set -DMIN_LOG_LEVEL=3 for release-type builds
This change implicitly adds IPv6 support.
If the address contains ":", we try to use a TCP socket instead of a Unix domain
socket. Everything in front of the last occurrence of ":" is the hostname and
everything after it the port.
If the hostname lookup fails, we fall back to using a Unix domain socket.
If the port is empty ("localhost:"), a random port will be assigned.
Examples:
NVIM_LISTEN_ADDRESS=localhost:12345 -> TCP (IPv4 or IPv6), port: 12345
NVIM_LISTEN_ADDRESS=localhost: -> TCP (IPv4 or IPv6), port: random (> 1024)
NVIM_LISTEN_ADDRESS=localhost:0 -> TCP (IPv4 or IPv6), port: random (> 1024)
NVIM_LISTEN_ADDRESS=localhost -> Unix domain socket "localhost" in current dir
Asynchronous API functions are served immediately, which means pending
input could change the state of Nvim shortly after an async API function
result is returned.
nvim_get_mode() is different:
- If RPCs are known to be blocked, it responds immediately (without
flushing the input/event queue)
- else it is handled just-in-time before waiting for input, after
pending input was processed. This makes the result more reliable
(but not perfect).
Internally this is handled as a special case, but _semantically_ nothing
has changed: API users never know when input flushes, so this internal
special-case doesn't violate that. As far as API users are concerned,
nvim_get_mode() is just another asynchronous API function.
In all cases nvim_get_mode() never blocks for more than the time it
takes to flush the input/event queue (~µs).
Note: This doesn't address #6166; nvim_get_mode() will provoke #6166 if
e.g. `d` is operator-pending.
Closes#6159
Also re-word some error messages:
- "Key does not exist: %s"
- "Invalid channel: %<PRIu64>"
- "Request array size must be 4 (request) or 3 (notification)"
- "String cannot contain newlines"
References #6150
