aboutsummaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAge
* Merge pull request #1431 from elmart/clang-analysis-fixes-2Justin M. Keyes2014-11-10
|\ | | | | Fix clang analysis warnings. (2)
| * Fix warnings: message.c: vim_vsnprintf(): Dead assignment (2): HI.Eliseo Martínez2014-11-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Problem : Dead assignment @ 3323. Dead assignment @ 3587. Diagnostic : Harmless issues. Rationale : - 3323: Assignment is in fact dead. But, in addition to that, `length_modifier` is assigned default value `\0` when declared and is untouched in path leading to signaled point. So, maintaining assignment adds nothing to code. - 3587: Assignment is in fact dead. It could be thought that `precision_specified` has to be 1 in order to flag `precision` as having a valid value. But that doesn't seem to be the case, as there are places in the code where `precision` gets assigned a default value, even if `precision_specified` is 0. So, maintaining assignment adds nothing to code. Resolution : Remove dead assignments.
| * Fix warnings: message.c: delete_first_msg(): Np dereference: FP.Eliseo Martínez2014-11-11
| | | | | | | | | | | | | | | | | | | | | | Problem : Dereference of null pointer @ 693. Diagnostic : False positive. Rationale : Error condition occurs if `delete_first_msg` is entered two consecutive times, the firt of which sets leaves history empty. But, in that case, second entrance should leave at the `return FAIL`, and thus cannot reach the pointer dereference. Resolution : Assert history will be empty after first entrance.
| * Fix warnings: memory.c: xcalloc(): 0 size calloc: CW.Eliseo Martínez2014-11-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Problem : Undefined allocation of 0 bytes (CERT MEM04-C; CWE-131) @ 148. Diagnostic : Cautionary warning. Rationale : Reported circumstance (calling calloc with requesting 0 size allocation) can occur, and it's not an issue. It's perfectly legal doing so, though result is implementation dependant. A given implementation can return NULL or a valid pointer, so that free() can be called on it later the same as if it was a real pointer. Such a pointer should not be dereferenced, though. Now, for some reason I can't explain, compiler is warning us in the case of calloc, but not in the case of malloc, which is doing the same. Resolution : Refactor memory functions to avoid using implementation dependant behaviour. Proposed code is neater to read, and it avoids calling system memory functions with size 0, thus behaving the same everywhere. Note that semantics for xmalloc/xcalloc is slightly changed: - Previously, an implementation that returns a valid pointer on malloc/calloc with 0 size, would return that pointer to xmalloc/xcalloc caller. - Currently, a regular pointer is always returned.
| * Fix warnings: hardcopy.c: mch_print_text_out(): Bad free: FP + RI.Eliseo Martínez2014-11-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Problem : Bad free @ 3058. Diagnostic : False positive uncovering a real issue. Rationale : Signaled error occurs if p gets assigned `(char_u*)""` at line 3009 and then is freed at line 3058. But that cannot happen because of the last guard condition before `free` (`*p != NUL`). So, signaled error is a false positive. Now, examining this code more carefully reveals a real issue: not freeing an empty string may not be always correct, as an empty (but allocated) string could also be returned in `p = string_convert(&prt_conv, p, &len);` if passed '&len' points to 0). Which would in fact be a memory leak. Resolution : Remove the exceptional case. Make p always point to allocated memory, and always free it, when `prt_do_conv` is on.
| * Fix warnings: fileio.c: aucmd_prepbuf(): Np dereference: FP.Eliseo Martínez2014-11-11
| | | | | | | | | | | | | | | | | | | | Problem : Dereference of null pointer @ 6219. Diagnostic : False positive. Rationale : Problem occurs if `aucmd_win` is NULL after `win_alloc_aucmd_win()`, which cannot happen since it uses new memory functions. So, this is a leftover since OOM refactorization. Resolution : Remove dead code.
| * Fix warnings: fileio.c: readfile(): Dead assignment: HI.Eliseo Martínez2014-11-11
| | | | | | | | | | | | | | | | | | | | | | Problem : Dead assignment @ 1754. Diagnostic : Harmless issue. Rationale : It's true `iconv_fd` is not going to be used again (we are in the failure handler). But what is being done (assigning sentinel value to mark as "empty" after destroying) is in fact good practice, which could turn significant if more code is added later on. So, we don't want to remove this. Resolution : Leave it there, but exclude from analysis.
| * Fix warnings: fold.c: get_foldtext(): Np dereference: FP.Eliseo Martínez2014-11-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Problem : Dereference of null pointer @ 1701. Diagnostic : False positive. Rationale : Comparison `last_wp != wp` just after initializing `last_wp` to NULL makes the compiler think `wp` can be null. Error appears then on codepath assuming comparison is false (i.e. `wp` is null). Resolution : Change order of OR clauses. That seems not to give motives for the analyzer to check the `wp` null path and removes the warning. But potential null dereference is still there, so we add the nonnull annotation to `wp` parameter.
| * Fix warnings: ex_getln.c: init_history(): Double free: FP.Eliseo Martínez2014-11-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
| * Fix warnings: ex_eval.c: report_pending(): Np dereference: FP.Eliseo Martínez2014-11-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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`.
| * Fix warnings: ex_docmd.c: eval_vars(): Unitialized arg: FP.Eliseo Martínez2014-11-11
| | | | | | | | | | | | | | | | | | 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.
| * Fix warnings: ex_cmds2.c: do_source(): Unitialized arg (2): MI.Eliseo Martínez2014-11-11
| | | | | | | | | | | | | | | | | | 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.
| * Fix warnings: ex_cmds2.c: do_source(): Np dereference: FP.Eliseo Martínez2014-11-11
| | | | | | | | | | | | | | | | | | 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).
| * Fix warnings: ex_cmds.c: do_ecmd(): Np dereference: FP.Eliseo Martínez2014-11-11
| | | | | | | | | | | | | | Problem : Dereference of null pointer @ 2768. Diagnostic : False positive. Rationale : `win_valid(oldwin)` implies `oldwin` not null. Resolution : Assert `oldwin` not null.
| * Fix warnings: ex_cmds.c: do_ascii(): Garbage value (2): MI.Eliseo Martínez2014-11-11
|/ | | | | | | | | 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.
* Merge pull request #1448 from tarruda/shell-systemlist-test-fixesJustin M. Keyes2014-11-10
|\ | | | | Fixes to shell.c, systemlist and functional tests
| * shell: Use job_write_cb for closing stdinThiago de Arruda2014-11-10
| | | | | | | | | | | | | | | | | | | | 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.
| * eval: Return an empty list from systemlist() when there's no outputThiago de Arruda2014-11-10
| | | | | | | | This is the behavior on vim's `systemlist()`.
| * test: Fix nondeterminism in tests with notificationsThiago de Arruda2014-11-10
|/ | | | | | | | Tests which spin the event loop and stop it in a notification handler have a chance of re-entering the event loop due to the `vim_eval` call in the `request()` helper(assuming the request call is what triggered the notification). Since this will cause an error to be thrown by the lua client, don't send the extra `vim_eval` request when the loop has been stopped.
* Merge pull request #1416 from jondkinney/masterJustin M. Keyes2014-11-10
|\ | | | | Fix the nvim-clipboard help instructions
| * Fix the nvim-clipboard help instructionsJon Kinney2014-11-06
| | | | | | | | Fixes #1407
* | Merge pull request #1442 from fwalch/improve-legacy2luatestJustin M. Keyes2014-11-10
|\ \ | | | | | | Improve legacy2luatest script.
| * | Improve legacy2luatest script.Florian Walch2014-11-09
| | |
* | | Merge pull request #1443 from jszakmeister/allow-root-dir-installsJohn Szakmeister2014-11-09
|\ \ \ | | | | | | | | build: allow installing into the root directory (/)
| * | | build: allow installing into the root directory (/)John Szakmeister2014-11-09
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It turns out that CMake always canonicalizes `CMAKE_INSTALL_PREFIX` to an absolute path--if it's a relative path, it canonicalizes it relative to the build directory. As a result, the only thing the DESTDIR and relative directory check prevents is an installation into the root directory since CMake strips the trailing slash, turning "/" into an empty string. Let's just remove the check all together, since it cannot accomplish what we intended.
* | | | Merge pull request #1429 from oni-link/fix.job_start.leakJustin M. Keyes2014-11-09
|\ \ \ \ | |/ / / |/| | | Fix memory leak in job_start().
| * | | Try to fix problem found in the Travis Ci build.oni-link2014-11-09
| | | | | | | | | | | | | | | | | | | | | | | | | | | | An uv_pipe_t handle for a WStream could be left open for a particular code path. Patch by tarruda.
| * | | Mark some function arguments as [consumed] in the docs.oni-link2014-11-09
| | | | | | | | | | | | | | | | | | | | The argument argv of job_start() and channel_from_job() will be freed. Mark them as such in the comments of this functions.
| * | | job: Fix memory leak in job_start().oni-link2014-11-09
| | | | | | | | | | | | | | | | | | | | If a new job cannot be started because no slots are free, we return early without freeing the argv argument.
* | | | Merge pull request #1441 from jszakmeister/remove-redundant-defineJohn Szakmeister2014-11-09
|\ \ \ \ | | | | | | | | | | Remove redundant USE_ICONV define from config.h.in.
| * | | | Remove redundant USE_ICONV define from config.h.in.John Szakmeister2014-11-09
| | | | | | | | | | | | | | | | | | | | This was noticed during a review of #1437.
* | | | | Merge pull request #1403 from fwalch/version-flagsJustin M. Keyes2014-11-09
|\ \ \ \ \ | | | | | | | | | | | | version: Add compilation info.
| * | | | | version: Add compilation info.Florian Walch2014-11-09
| |/ / / /
* | | | | Merge pull request #1389 from elmart/clang-analysis-fixesJustin M. Keyes2014-11-09
|\ \ \ \ \ | |/ / / / |/| | | | Fix clang analysis warnings.
| * | | | Fix warnings: edit.c: ins_bs(): Garbage result: MI.Eliseo Martínez2014-11-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
| * | | | Fix warnings: edit.c: replace_do_bs(): Garbage value: MI.Eliseo Martínez2014-11-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
| * | | | Fix warnings: edit.c: mb_replace_pop_ins(): Unitilialized arg: FP.Eliseo Martínez2014-11-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
| * | | | Fix warnings: edit.c: ins_compl_get_exp(): Np dereference (2): FP.Eliseo Martínez2014-11-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
| * | | | Fix warnings: edit.c: ins_compl_next_buf(): Np dereference: FP.Eliseo Martínez2014-11-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
| * | | | Fix warnings: regexp.c: match_with_backref(): Nonnull violation: FP.Eliseo Martínez2014-11-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
| * | | | Fix warnings: regexp.c: br_regcomp(): Np dereference: MI.Eliseo Martínez2014-11-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
| * | | | Fix warnings: regexp.c: skip_regexp: Np dereference: FP.Eliseo Martínez2014-11-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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).
| * | | | Fix warnings: ops.c: cursor_pos_info(): Various (2): MI.Eliseo Martínez2014-11-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
| * | | | Fix warnings: ops.c: do_join(): Nonnull violation: FP.Eliseo Martínez2014-11-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
| * | | | Fix warnings: screen.c: draw_tabline(): Dead assignment: HI.Eliseo Martínez2014-11-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
| * | | | Fix warnings: screen.c: showmode(): Dead assignment: HI.Eliseo Martínez2014-11-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
| * | | | Fix warnings: screen.c: win_line(): Dead assigment: HI.Eliseo Martínez2014-11-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
| * | | | Fix warnings: screen.c: win_line(): Dead initialization: HI.Eliseo Martínez2014-11-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
| * | | | Fix warnings: screen.c: redraw_asap(): Various (6): MI.Eliseo Martínez2014-11-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
| * | | | Fix warnings: cursor_shape.c: parse_shape_opt(): Garbage value: FP.Eliseo Martínez2014-11-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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`.
generated by cgit (git 2.34.1) at 2025-06-01 18:48:15 +0000