Coverity#4183
Conversation
|
I have a few concerns about asserting that I wonder if we could set a thread-local variable when these fail and check it again later? I don't have a good feeling for how ugly it would be to do that after-the-fact checking, though. |
|
I definitly agree regarding the locking. What is really to blame here is the OpenSSL API, which does not allow the locking function to return an error. The workaround with a thread-local variable might work, but I do not know off-hand which call paths may invoke the locking function at all. I'm a bit clueless on the design of this part of OpenSSL. |
The current code in `parse_section_header_ext` is only prepared to properly handle out-of-memory conditions for the `git_buf` structure. While very unlikely and probably caused by a programming error, it is also possible to run into error conditions other than out-of-memory previous to reaching the actual parsing loop. In these cases, we will run into undefined behavior as the `rpos` variable is only initialized after these triggerable errors, but we use it in the cleanup-routine. Fix the issue by unifying the function's cleanup code with an `end_error` section, which will not use the `rpos` variable.
In the `_check_dir_contents` function, we first allocate memory for joining the directory and subdirectory together and afterwards use `git_buf_joinpath`. While this function in fact should not fail as memory is already allocated, err on the safe side and check for returned errors.
Short-circuit the call to `git_path_resolve_relative` in case `git_buf_joinpath` returns an error. While this does not fix any immediate errors, the resulting code is easier to read and handles potential new error conditions raised by `git_buf_joinpath`.
We do not check the return value of `git__calloc`, which may return `NULL` in out-of-memory situations. Fix the error by using `GITERR_CHECK_ALLOC`.
When executing `git_futils_mmap_ro_file`, we first try to guess whether the file is mmapable at all. Part of this check is whether the file is too large to be mmaped, which can be true on systems with 32 bit `size_t` types. The check is performed by first getting the file size wtih `git_futils_filesize` and then checking whether the returned size can be represented as `size_t`, returning an error if so. While this test also catches the case where the function returned an error (as `-1` is not representable by `size_t`), we will set the misleading error message "file too large to mmap". But in fact, a negative return value from `git_futils_filesize` will be caused by the inability to fstat the file. Fix the error message by handling negative return values separately and not overwriting the error message in that case.
|
I've removed the OpenSSL-specific code for now. OpenSSL supports user error codes via |
Another set of Coverity fixes. Some of these are mostly cosmetic fixes, some are actual fixes for rather unlikely error cases.