* refactor: disable formatting for attribute in macro
* fixup: disable/enable uncrustify with uncrustify:indent-off/on
* fixup: stop indenting contents inside braces in case
* fixup: remove case brace if no variable declaration
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.
- 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
Current name is inappropriate for the following reasons:
1. It is often masked by local `loop` variables.
2. It cannot be searched for. There are many `loop` variables where `loop` is
some local variable. There are many cases when “loop” word is used in
a comment.
3. It is in any case bad idea to use a generic name as a name of the global
variable. Best if global has module prefix: this is why it is in `main.h`:
`main_loop` both stands for “a main loop” and “a loop defined in `main.*`”.
Since I have no idea how to list every occurrence of this variable method used
to rename it is “remove it from globals.h, try to compile, fix errors”. Thus if
some occurrence was hidden under false `#if` branch it was not replaced.
Return 1 if the endpoint argument is NULL, server_start() can get
a NULL value when using server_address_new() or vim_tempname(). Removed
the function attribute.
When creating a local socket/pipe (server_start()) Neovim used vim_tempname() to
generate a unique socket path. For Windows UNIX filepaths cannot be used as
pipe names (they must start with \\.\pipe\). This commit replaces the use of
vim_tempname() for server addresses with server_address_new().
server_address_new() generates unique names for local sockets/pipes - for UNIX
it uses vim_tempname(), for Windows generates names in the form
\\.\pipe\nvim-PID-COUNTER
where PID is the current process id, and COUNTER is a static uint32_t counter
incremented with every call. This function is now used for server_start() and
server_init() when no address is available.
- Add event loop abstraction module under src/nvim/event. The
src/nvim/event/loop module replaces src/nvim/os/event
- Remove direct dependency on libuv signal/timer API and use the new abstraction
instead.
- Replace all references to uv_default_loop() by &loop.uv, a new global variable
that wraps libuv main event loop but allows the event loop functions to be
reused in other contexts.
Making an environment variable empty can be a way of unsetting it for
platforms that don't support unsetenv(). In most cases, we treat empty
variables as having been unset. For all others, use os_env_exists().
- On startup, v:servername is equivalent to $NVIM_LISTEN_ADDRESS
- v:servername may be considered the "default" server address
- v:servername does not change unless the associated server is stopped
by serverstop()
suspicious_sizeof: Passing argument 8UL /* sizeof (char const **) */ to
function xcalloc and then casting the return value to char ** is
suspicious.
In this particular case sizeof (char const **) happens to be
equal to sizeof (char const *), but this is not a portable as
Implement functions for spawning, destroying, and listing active
servers, and add server_address_list() to msgpack_rpc/server.c for the
serverlist() vimL function.
Testing of server_start() and server_stop() found that after adding a
server at address addr, pmap_has(addr) would always return true, but
pmap_get(addr) would always return NULL.
Since a client is only expected to have a small number of servers, an
array may be more efficient than a hash map, anyway.
Discussion:
https://github.com/neovim/neovim/pull/1302#issuecomment-88487148
We already use wrappers for allocation, the new `xfree` function is the
equivalent for deallocation and provides a way to fully replace the malloc
implementation used by Neovim.
If the server fails to start(due to used address for example), the
`server_start` function was freeing the handle memory before it was properly
removed from libuv event loop queue. Fix that by replacing the `free(server)`
call by `uv_close` call, which will take care of freeing the server on the next
event loop iteration. Also replace `EMSG` calls by `ELOG`/`WLOG`.
All files under the os, api and msgpack_rpc directories have -Wconversion
automatically applied. CONV_SOURCES is also checked for missing files(when
renaming, for example)
