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.
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.
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.
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.
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.
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.
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.
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).
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.
Fixes#1447. `CMAKE_MODULE_PATH` is meant to be a list of directories,
and as such, is not the proper way to launch our scripts. Let's use
`${PROJECT_SOURCE_DIR}/cmake` instead. Also, let's not outright set
`CMAKE_MODULE_PATH`, but instead append our location to the list.
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.
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.
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.
Commit a1d411f9c9 just assumes that gcc
will support the `-Og` option, but gcc that comes with Ubuntu 12.04 does
not. Let's check to see if the flag is supported, and then decide
whether to enable it or not.
They're unnecessary for the rest of the file, and they're only there at
all to help implement `free_all_mem` for use with Clang's Address
Sanitizer. So let's move them to avoid any confusion about why they are
there.
It turns out that `file(INSTALL ...)` already accounts for `DESTDIR`, so
this wasn't creating the directory structure in the correct location.
Instead, we need to do our existence check with `DESTDIR`, but leave it
off when doing the install step.
While we're at it, add a check to make sure `ENV{DESTDIR}` is not being
used with a relative path, as that construct doesn't make much sense.
This fixes issue #1387 discovered while trying to make helptag
generation work correctly in #1381.
Unfortunately, we can't force the specific inclusion of a header file.
So if anything add /opt/local/include to the include path--such as
libintl--then other dependencies might be drawn from /opt/local at
compile time, even though we detected them elsewhere at configure time.
This, in turn, causes issues with mixed versions, such as the iconv.h
header being pulled from /opt/local/include, but linked against the
library in /usr/lib--which can be mismatched versions.
So, despite CMake's best effort to treat /sw and /opt/local as just
another system area, we really need to give them preferential treatment.
To do this, we add them to CMAKE_PREFIX_PATH.
This fixes an issue discovered while re-enabling iconv in #1370.
