Problem : getdigits() currently returns a long, but at most places,
return value is casted (unsafely) into an int. Making casts
safe would introduce a lot of fuss in the form of assertions
checking for limits.
Note : We cannot just change return type to int, because, at some
places, legitimate long values are used. For example, in
diff.c, for line numbers.
Solution : Introduce new functions:
- get_digits() : Gets an intmax_t from a string.
- get_int_digits() : Wrapper for ints.
- get_long_digits() : Wrapper for longs.
And replace getdigits() invocations by the appropiate
wrapper invocations.
For garbage collection all lists are kept in first_list, a list of all
lists.
free_unref_items() searches through first_list and removes unreferenced
lists from it (by calling list_free(..., FALSE)). But after a list was
removed, the search continues from the beginning of first_list (not sure
how many lists were really removed and where to continue in first_list).
This is not necessary anymore since vim-patch 7.0.135, because a call to
list_free(...,FALSE) makes sure, that no other lists (and dictionaries)
are freed. So we always know, that the next list in first_list is
still valid (allocated or NULL) and can be used to continue the search.
Likewise for dictionaries.
Original patch by Ariya Mizutani
https://groups.google.com/forum/#!searchin/vim_dev/GC/vim_dev/DBYOdHQWvqY/1WH04_dwETIJ
Problem: gettabvar() is not consistent with getwinvar() and getbufvar().
Solution: Return a dict with all variables when the varname is empty.
(Yasuhiro Matsumoto)
https://code.google.com/p/vim/source/detail?r=v7-4-434
A similar macro is defined in the Linux kernel [1].
To refactor the code I used a slightly modified Coccinelle script I found in
[2].
```diff
// Use the macro ARRAY_SIZE when possible
//
// Confidence: High
// Copyright: (C) Gilles Muller, Julia Lawall, EMN, DIKU. GPLv2.
// URL: http://www.emn.fr/x-info/coccinelle/rules/array.html
// Options: -I ... -all_includes can give more complete results
@@
type T;
T[] E;
@@
- (sizeof(E)/sizeof(*E))
+ ARRAY_SIZE(E)
@@
type T;
T[] E;
@@
- (sizeof(E)/sizeof(E[...]))
+ ARRAY_SIZE(E)
@@
type T;
T[] E;
@@
- (sizeof(E)/sizeof(T))
+ ARRAY_SIZE(E)
@n@
identifier AS,E;
@@
- #define AS(E) ARRAY_SIZE(E)
@@
expression E;
identifier n.AS;
@@
- AS(E)
+ ARRAY_SIZE(E)
```
`spatch --in-place --sp-file array_size.cocci -I src/ -I build/include/ -I build/src/nvim/auto/ src/nvim/*.c`
[1] http://lxr.free-electrons.com/source/include/linux/kernel.h#L54
[2] http://www.emn.fr/z-info/coccinelle/rules/#macros
Problem: getreg() does not distinguish between a NL used for a line
break and a NL used for a NUL character.
Solution: Add another argument to return a list. (ZyX)
https://code.google.com/p/vim/source/detail?r=v7-4-242
** CID 74786: Resource leak (RESOURCE_LEAK)
/src/nvim/eval.c: 10614 in f_jobsend()
/src/nvim/eval.c: 10616 in f_jobsend()
save_tv_as_string() should return NULL and input_len <= 0 for an empty
string or error. Callers should check that input != NULL instead of
input_len > 0 and assert(input == NULL) when the length must be checked.
Also:
- Remove NO_CONSOLE_INPUT/NO_CONSULE preprocessor conditionals
- Remove ctrl_c_interrupts variable, check for mapped_ctrl_c directly in
process_interrupts()
- Move ui_inchar profiling to input_poll which is where Nvim blocks for input.
- By default vim_feedkeys escaped all input for CSI/K_SPECIAL bytes
before using it. However since vim_replace_termcodes() also escapes
the input string chaining these functions together escapes input twice
- vim_feedkeys() now takes a third Boolean argument to enable/disable
escaping
- Breaks API compatibility
- get_system_output_as_rettv() was missing a refcount increment when
returning an empty list, i.e. when there was no output
- we now use rettv_list_aloc() instead of list_alloc()
- issue #1530
Problem : Assigned value is garbage or undefined @ 12578.
Diagnostic : Multithreading issue.
Rationale : Error can only occur if global `provider_call_nesting` is
changed while function is executing.
Resolution : Use local copy of global.
Problem : Dereference of null pointer @ 18841.
Diagnostic : False positive.
Rationale : Suggested error path takes `reanimate` branch at 18827,
assigning `rettv = current_funccal->rettv`. Then,
inmediately after, it supposes rettv is null, which cannot
happen, since current_funccal->rettv should always be non
null.
Resolution : Assert current_funccal->rettv non null.
Problem : Out-of-bound array access @ 18737.
Diagnostic : False positive.
Rationale : Situation is intentional. `dictitem_T` is a prefix all dict
items whill share, but actual size of each item will be
different depending on its key length. `di_key` array field
is declared of size 1 just to have a field name, but real
size will vary for each item.
Resolution : Make analyzer ignore it.
This could be refactored to use C99-allowed variable length
arrays, but eval.c is bound to dissappear, so no effort is
done in that sense.
Problem : Out-of-bound array access @ 18429.
Diagnostic : False positive.
Rationale : Situation is intentional. `dictitem_T` is a prefix all dict
items whill share, but actual size of each item will be
different depending on its key length. `di_key` array field
is declared of size 1 just to have a field name, but real
size will vary for each item.
Resolution : Make analyzer ignore it.
This could be refactored to use C99-allowed variable length
arrays, but eval.c is bound to dissappear, so no effort is
done in that sense.
Problem : Dereference of null pointer @ 18216.
Diagnostic : False positive.
Rationale : `hi` and `done` are static. Intended usage is for the first
call to have idx == 0, so that they are initialized.
Resolution : Assert hi after (optional) initialization.
Problem : Bad free @ 16076.
Diagnostic : Real issue.
Rationale : A non-allocated string is set at 4127, which later on can
be tried to be freed if aborting.
Resolution : Detect particular case (func with empty name) and don't
free in that case.
Another solution (use allocated string) was tried before,
but it produced a leak difficult to solve.
Finally applied solution works, but it produces a new false
positive warning (Np dereference at 13763), deactivated by
`assert(ptrs[i].item->li_next)`.
Problem : Result of operation is garbage or undefined @ 13565.
Diagnostic : Multithreading issue.
Rationale : Problem occurs only if global (static) variable
`item_compare_keep_zero` changes after being used by
`do_sort_uniq` but before being used by `item_compare` or
`item_compare2`.
Resolution : This is not an intra-function problem, as other MI's
before, but rather an inter-function one. Thus, it can't be
solved by using local copy of global. Therefore, we are
forced to do a bit refactoring. We can't simply add a bool
param to item_compare/item_compare2, as they couldn't be
passed to qsort() that way. So, item_compare/item_compare2
are added a bool param and curried versions of them are
added and used in their place.
Problem : Out-of-bound array access @ 5737.
Diagnostic : False positive.
Rationale : Situation is intentional. `dictitem_T` is a prefix all dict
items whill share, but actual size of each item will be
different depending on its key length. `di_key` array field
is declared of size 1 just to have a field name, but real
size will vary for each item.
Resolution : Make analyzer ignore it.
This could be refactored to use C99-allowed variable length
arrays, but eval.c is bound to dissappear, so no effort is
done in that sense.
Problem : Dereference of null pointer @ 2273.
Diagnostic : False positive.
Rationale : Suggested error would happen when assigning an rvalue with
more items than the lvalue. Then we would enter conditional
at:
```
if (lp->ll_li->li_next == NULL) {
/* Need to add an empty item. */
list_append_number(lp->ll_list, 0);
}
lp->ll_li = lp->ll_li->li_next;
```
Analyzer thinks the value assigned to lp->ll_li is still
NULL and is hit on the next iteration.
Resolution : Assert lp->ll_li->li_next is not null anymore after
list_append_number().
These use autoloaded vimscript to replace the provider_call/provider_has
functions, moving the implementation of providers to pure vimscript(we lose
nothing since vimscript can also call msgpack-rpc functions).
When calling the rpcrequest function from a provider, temporarily switch to the
caller scope. This is required for compatibility with legacy plugins, because
they may depend on scope information that changes when "leaving" the C stack to
enter the vimscript stack.
