Allow Include What You Use to remove unnecessary includes and only
include what is necessary. This helps with reducing compilation times
and makes it easier to visualise which dependencies are actually
required.
Work on https://github.com/neovim/neovim/issues/549, but doesn't close
it since this only works fully for .c files and not headers.
Problem: Macros for MS-Windows are inconsistent, using "32", "3264 and
others.
Solution: Use MSWIN for all MS-Windows builds. Use FEAT_GUI_MSWIN for the
GUI build. (Hirohito Higashi, closesvim/vim#3932)
4f97475d32
Problem:
- Since c57f6b28d7#8519, sockets are created in ~/.local/… but XDG
spec says: "XDG_RUNTIME_DIR: Must be on the local filesystem", which
implies that XDG_STATE_DIR is potentially non-local.
- Not easy to inspect Nvim-created temp files (for debugging etc).
Solution:
- Store sockets in stdpath('run') ($XDG_RUNTIME_DIR).
- Establish "/tmp/nvim.user/" as the tempdir root shared by all Nvims.
- Make ok() actually useful.
- Introduce assert_nolog().
closes#3517closes#17093
Problem:
- Unix sockets are created in random /tmp dirs.
- /tmp is messy, unclear when OSes actually clear it.
- The generated paths are very ugly. This adds friction to reasoning
about which paths belong to which Nvim instances.
- No way to provide a human-friendly way to identify Nvim instances in
logs or server addresses.
Solution:
- Store unix sockets in stdpath('state')
- Allow --listen "name" and serverstart("name") to given a name (which
is appended to a generated path).
TODO:
- is stdpath(state) the right place?
Problem:
1. Log messages (especially in CI) are hard to correlate with tests.
2. Since b353a5c05f#11886, dumplog() prints the logs next to test
failures. This is noisy and gets in the way of the test results.
Solution:
1. Associate an incrementing id with each test and include it in log
messages.
- FUTURE: add v:name so Nvim instances can be formally "named"?
2. Mention "child" in log messages if the current Nvim is a child (based
on the presence of $NVIM).
BEFORE:
DBG … 12345 UI: event
DBG … 12345 log_server_msg:722: RPC ->ch 1: …
DBG … 12345 UI: flush
DBG … 12345 inbuf_poll:444: blocking... events_enabled=1 events_pending=0
DBG … 23454 UI: stop
INF … 23454 os_exit:594: Nvim exit: 0
AFTER:
DBG … T57 UI: event
DBG … T57 log_server_msg:722: RPC ->ch 1: …
DBG … T57 UI: flush
DBG … T57 inbuf_poll:444: blocking... events_enabled=1 events_pending=0
DBG … T57/child UI: stop
INF … T57/child os_exit:594: Nvim exit: 0
PROBLEM
------------------------------------------------------------------------
$NVIM_LISTEN_ADDRESS has conflicting purposes as both a parameter ("the
current process should listen on this address") and a descriptor ("the
current process is a child of this address").
This contradiction means the presence of NVIM_LISTEN_ADDRESS is
ambiguous, so child Nvim always tries to listen on its _parent's_
socket. This is the cause of lots of "Failed to start server" spam in
our test/CI logs:
WARN 2022-04-30… server_start:154: Failed to start server: address already in use: \\.\pipe\nvim-4480-0
WARN 2022-04-30… server_start:154: Failed to start server: address already in use: \\.\pipe\nvim-2168-0
SOLUTION
------------------------------------------------------------------------
1. Set $NVIM to the parent v:servername, *only* in child processes.
- Now the correct way to detect a "parent" Nvim is to check for $NVIM.
2. Do NOT set $NVIM_LISTEN_ADDRESS in child processes.
3. On startup if $NVIM_LISTEN_ADDRESS exists, unset it immediately after
server init.
4. Open a channel to parent automatically, expose it as v:parent.
Fixes#3118Fixes#6764Fixes#9336
Ref https://github.com/neovim/neovim/pull/8247#issuecomment-380275696
Ref #8696
* 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)
