Use save_tv_as_string(), same as vimL system(). This also makes
jobsend() more liberal in what it can accept. For example,
`jobsend(j, 123)` is now valid.
Closes#1176
Factor out string_to_list() from f_system()'s implementation
and use that to set job_data. This has the technical advantage of
preserving NULs, and may be more convenient for users.
Required for #1176.
The -O3 optimization level can often lead to dangerous (and sometimes
incorrect) optimizations being performed. So let's use a level that's
more stable.
During test setup, we used to call a vimscript function(BeforeEachTest) that
attempted to restore Nvim to it's initial state as much as possible in order to
provide a clean environment for running new tests. This approach has proven to
be unreliable, as some tests leave state that can affect other tests, eventually
causing failures that are difficult to debug.
This commit changes the 'clear' function so it will restart Nvim every time it
is called, which is a slower, but more reliable solution that will simplify
spotting bugs in the future.
Some other improvements/fixes were also performed:
- Whenever an error is detected in a handler passed to "run()", the event loop
will be stopped and the error will be propagated to the main thread.
- Errors and the "cleanup()" function will always send a quit command to the
current Nvim instance. This should prevent memory starvation when running
tests under valgrind(where each Nvim instance can consume a lot of memory).
- Fixed a wrong assertion in server_requests_spec.lua. Previously the failure
was undetected in a notification handler.
- Fixed some tests to expect fully clean registers. The deleted cleanup function
used to put an empty string in every register, but that resulted in a extra
line being added.
- memfile_defs.h:
* hashtab_T: mht_mask: long_u -> size_t.
Masks are used to truncate keys to a value adequate for an index
in the array of entries. Value of the mask plus one is the
current size of the array. Both of those reasons imply the
soundness of size_t for this type.
* hashtab_T: mht_count: long_u -> size_t.
- memfile.c:
* total_mem_used: long_u -> size_t.
* mf_hash_free_all: idx: long_u -> size_t.
* mf_hash_add_item: idx: long_u -> size_t.
* mf_hash_find: idx: long_u -> size_t.
* mf_hash_grow: i: long_u -> size_t.
* mf_hash_grow: j: long_u -> size_t.
- Drop '_S' suffix for struct names.
- Make struct names match corresponding type's name (just removing '_S' suffix).
- Rename NR_TRANS type/struct (just ugly).
memfile_defs.h:
- Inline struct definitions in typedefs.
- Move memfile_T definition to this file (weirdly, was in buffer_defs.h).
memfile.c:
- Use C99 style variable declarations. This is, move variable declarations as
near to first-usage point as possible).
- Modernize old-style function declarations.
- Fix indent at some places (some multiline expressions and the like).
Problem : Result of operation is garbage or undefined @ 7460.
Diagnostic : Multithreading issue.
Rationale : Problem occurs if any of globals `enc_utf8`, `p_deco is
modified while function is executing.
Resolution : Use local copy of globals.
Problem : Assigned value is garbage or undefined @ 6359.
Diagnostic : Multithreading issue.
Rationale : Problem only occurs if global `State` changes while
function is executing.
Resolution : Use local copy of global in function.
Problem : Uninitialized argument value @ 6296.
Diagnostic : False positive.
Rationale : Error occurs if n <= 1. That's not possible because
n >= 1 due to `MB_BYTE2LEN` postcondition and n != 1
because we are in the else branch.
Resolution : Assert n > 1.
Problems : Dereference of null pointer @ 3615.
Dereference of null pointer @ 3764.
Diagnostic : False positives.
Rationale : `ins_buf` is local static, so maintains value between calls.
This function will be called first when `compl_started` is
false, and in that case it initializes `ins_buf`. After
that, it can be called multiple times with `compl_started`
true, where `ins_buf` will be updated but not to null.
So, when arriving to both points, `ins_buf` should never be
null.
Resolution : Assert `ins_buf` at both points.
Problem : Dereference of null pointer @ 3234.
Diagnostic : False positive.
Rationale : `wp` is local static, so maintains value between calls.
First time function is called for a given flag will have
`buf == curbuf`, implying `wp` initialization.
Resolution : Assert variable always having been initialized.
Problem: Argument with 'nonnull' attribute passed null @ 5632.
http://neovim.org/doc/reports/clang/report-041a0e.html#EndPath.
Diagnostic: False positive.
Rationale : `p = reg_getline(clnum)` above should not be null, because
`clnum` starts at `start_lnum` and only increments after
that.
Resolution: Assert p not null.
Problem: Dereference of null pointer @ 1312.
http://neovim.org/doc/reports/clang/report-b1d09a.html#EndPath
Diagnostic: Multithreading issue.
Rationale : Suggested error path contains two succesive calls to
`regnext(scan)`, first of which returning nonnull, the
second one returning null. This can only occur if global
`reg_toolong` accesed in `regnext()` changes between the
calls.
Resolution: Use local variable to cache first `regnext(scan)` result.
Note that this change alters function semantics, as now
function only issues one call instead of two, reusing the
result for the second time.
This shouldn't be a problem, though, as new semantics should
be in fact be better.
Problem: Derefence of null pointer @ 1208.
http://neovim.org/doc/reports/clang/report-24b5ca.html#Path10
Diagnostic: False positive.
Rationale : Error is reported to happen if after `if (*newp == NULL) {`
body, `*newp` continues being NULL, and false branch of
following `if (*newp != NULL)` is taken. Now, `vim_strsave`
cannot return NULL, so error cannot happen.
Resolution: Remove dead code (leftover since OOM refactors).
Problems: Result of operation is garbage or undefined @ 5087.
http://neovim.org/doc/reports/clang/report-2e3118.html#EndPath
Result of operation is garbage or undefined @ 5149.
Diagnostic: Multithreading issues.
Rationale : All reported problems can only occur if accesed globals
change state while executing function, which could only
happen in a multithreaded environment.
Resolution: Use local variables (copy globals on entry).
Note that this change alters function semantics, as now
function only depends on global values at entry time.
This shouldn't be a problem, though, as new semantics should
be in fact better.
Problem: Dead assignment @ 7711.
http://neovim.org/doc/reports/clang/report-835eb6.html#EndPath
Diagnostic: Harmless issue.
Rationale : `scol` is only used within `FOR_ALL_TABS` body, which
assigns another value to `scol` at the beginning of each
iteration. If `FOR_ALL_TABS` body would not execute (no
tabs) nothing harmful would happen, as code following
`FOR_ALL_TABS` doesn't use `scol`.
Resolution: Remove.
Problem: Dead assigment.
http://neovim.org/doc/reports/clang/report-7362ba.html#EndPath
Diagnostic: Harmless issue.
Rationale : `boguscols` is in fact unread by downstream code.
Resolution: Comment out. This is preferred here over just removing the
line because involved logic is complex, and future readers
of this code could find this extra knowledge useful to
understand what the code is doing.
Problem: Dead initialization @ 3477.
http://neovim.org/doc/reports/clang/report-94b736.html#EndPath
Diagnostic: Harmless issue.
Rationale : `len` is assigned a new value just some lines below. So,
this just seems something due to old-style variable
declarations.
Resolution: We could just remove initialization, but prefer moving
declaration down to point of initialization.
Problem: Assigned value is garbage or undefined @ 187.
http://neovim.org/doc/reports/clang/report-7b7d61.html#EndPath.
Diagnostic: False positive.
Rationale : `colonp`, must be `>= modep, or null` by `vim_strchr`
postcondition. At this point we also it's not null and it's
not equal to `modep`, by previous code. So, it must be
`> modep`.
Resolution: Assert `colonp > modep`.
