| Commit message (Collapse) | Author | Age |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Problem : Double free @ 4249.
Diagnostic : False positive.
Rationale : Codepath leading to error contains two consecutive
iterations in which `if (--j < 0)` is true.
That executes `free` two consecutive times with the same
value (hislen - 1) for j, with leads to double free.
Now, that can only happen with j == 0 && hislen == 1.
And that would imply j == hisidx[type] too, which would
take the following break.
So, the error codepath cannot really happen, but the
compiler cannot deduce the last implication.
Resolution : We have two possible solutions for this:
1.- Comparing value of j before and after updating it,
and breaking out of iteration if equal.
That changes nothing in functionality, but teaches the
compiler his proposed error codepath is impossible.
2.- Nullify pointer after freeing.
This way, the compiler still thinks his error codepath
is possible, but it's not an error anymore, as
free(NULL) is a no-op.
We opt for solution 2, as solution 1 requires adding
logic that adds nothing (and having to explain that clearly
in aside comments) just for the purpose of silencing
warning. On the other hand, solution 2 improves the code,
adding something considered good practice in any case,
and therefore doesn't require further explanation.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Problem : Dereference of null pointer @ 711.
Diagnostic : False positive.
Rationale : Codepath producing error invokes this function with values
`action=RPC_DISCARD, pending=CSTP_FINISH, value=NULL`.
Now, for some reason, the analyzer is remembering that
`value` is null, and that `action` is `RPC_DISCARD`, but
it's not remembering that `pending` is `CSTP_FINISH`.
Then, it's taking the wrong branch in the switch for
`pending`. That path would never occur invocating the
function with those values.
Resolution : Assert function precondition between `pending` and `value`.
This is, let the compiler know that `value` being null
implies `pending` not containing `CSTP_THROW`.
|
|
|
|
|
|
|
|
|
| |
Problem : Uninitialized argument value @ 7704.
Diagnostic : False positive.
Rationale : Error occurs if `switch(spec_idx)` doesn't enter any case,
which should not occur after
`spec_idx = find_cmdline_var(...)` returned non-negative.
Resolution : Add default clause to switch and error if reached.
|
|
|
|
|
|
|
|
|
| |
Problem : Uninitialized argument value @ 2485.
Uninitialized argument value @ 2507.
Diagnostic : Multithreading issues.
Rationale : Error can only occur if globals `do_profiling`, `time_fd`
are modified while function is executing.
Resolution : Use local copy of globals.
|
|
|
|
|
|
|
|
|
| |
Problem : Dereference of null pointer @ 2462.
Diagnostic : False positive.
Rationale : Error occurred if neither loop neither following if were
entered (this implied `script_items.ga_len < 0`, which
should not be possible).
Resolution : Assert not negative length (loop or if entered).
|
|
|
|
|
|
|
| |
Problem : Dereference of null pointer @ 2768.
Diagnostic : False positive.
Rationale : `win_valid(oldwin)` implies `oldwin` not null.
Resolution : Assert `oldwin` not null.
|
|
|
|
|
|
|
|
|
| |
Problems : Assigned value is garbage or undefined @ 127.
Assigned value is garbage or undefined @ 152.
Diagnostic : Multithreading issues.
Rationale : Error could only occurr if global `enc_utf8` changed while
the function is executing.
Resolution : Use local copy of global var.
|
|
|
|
|
|
|
|
|
|
| |
Commit @45525853d352 removed usage of the `job_write_cb` for closing stdin due
to a memory error, but that doesn't work anymore because `job_close_in` closes
stdin immediately, possibly trimming input data before it is fully written.
Since most memory issues with jobs have been fixed, re-add the `job_write_cb`
call to ensure stdin is only closed when it should. Also add tests for scenarios
where using the callback makes a difference.
|
|
|
|
| |
This is the behavior on vim's `systemlist()`.
|
|\
| |
| | |
Fix memory leak in job_start().
|
| |
| |
| |
| |
| |
| |
| | |
An uv_pipe_t handle for a WStream could be left open for a
particular code path.
Patch by tarruda.
|
| |
| |
| |
| |
| | |
The argument argv of job_start() and channel_from_job() will be
freed. Mark them as such in the comments of this functions.
|
| |
| |
| |
| |
| | |
If a new job cannot be started because no slots are free, we return early
without freeing the argv argument.
|
|\ \
| | |
| | | |
version: Add compilation info.
|
| | | |
|
|\ \ \
| |/ /
|/| | |
Fix clang analysis warnings.
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
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: Argument with 'nonnull' attribute passed null @ 3540.
http://neovim.org/doc/reports/clang/report-fc14e0.html#EndPath.
Diagnostic: False potitive.
Rationale : `count` should be >= 2 by function precondition.
Resolution: Assert precondition.
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
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 assignment @ 7535.
http://neovim.org/doc/reports/clang/report-19a5cd.html#EndPath
Diagnostic: Harmless issue.
Rationale : `length = msg_col;` is unconditionally executed after this.
Resolution: Remove assignment.
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
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.
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Problems: Argument with 'nonnull' attribute passed null @ 277.
http://neovim.org/doc/reports/clang/report-9c3614.html#EndPath
Result of operation is garbage or undefined @ 281.
http://neovim.org/doc/reports/clang/report-45efbf.html#EndPath
Argument with 'nonnull' attribute passed null @ 306.
http://neovim.org/doc/reports/clang/report-ffb84f.html#EndPath
Result of operation is garbage or undefined @ 311.
http://neovim.org/doc/reports/clang/report-d04333.html#EndPath
Argument with 'nonnull' attribute passed null @ 315.
http://neovim.org/doc/reports/clang/report-786819.html#EndPath
Uninitialized argument value @ 328.
http://neovim.org/doc/reports/clang/report-2a5506.html#EndPath
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.
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: 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`.
|
| |/
|/|
| |
| |
| | |
This will provide better control for those who may want to alter which
one gets used.
|
| |
| |
| |
| |
| |
| | |
- When an invalid msgpack RPC msg is received from a channel
we now close that channel all calls on that channel fail with
an error message.
|
| | |
|
| | |
|
| |
| |
| |
| |
| | |
- When validating a msgpack msg we need to return on the first error
otherwise we can SEGFAULT with invalid checks
|
| |
| |
| |
| |
| |
| |
| |
| | |
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.
|
| |
| |
| |
| | |
Only read up to the last newline in push_job_event().
|
| |
| |
| |
| |
| | |
Needed for accessing this information without direct access to the
rstream_buffer.
|
| | |
|
| |
| |
| |
| |
| | |
stdout/stderr should only be closed after the job truly exits, or else we can
lose data sent by it.
|
| | |
|
| |
| |
| |
| |
| | |
Replace long with int64_t.
Apart from being convenient, not using long is required by clint.
|
| |
| |
| |
| | |
Replace int with bool where appropriate.
|
| |
| |
| |
| |
| | |
This, in addition to being more correct, allows removing a lot of
explicit casts.
|
| |
| |
| |
| | |
Unused since 457bb2615154946d273d75e07f5d5a936f50ede0.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
- 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.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
- Add memfile.c to converted files list.
- Fix conversion issues:
* bhdr_T : bh_page_count : int -> unsigned.
* bhdr_T : bh_flags : char -> unsigned.
* mf_new() : page_count : int -> unsigned.
* mf_get() : page_count : int -> unsigned.
* mf_release() : page_count : int -> unsigned.
* mf_alloc_bhdr() : page_count : int -> unsigned.
* mf_trans_add() : page_count : int -> unsigned.
* mf_put() : flags : int -> unsigned.
|