We commonly have to check if a git_buf has been allocated
correctly or if we ran out of memory. Introduce a new macro
similar to `GITERR_CHECK_ALLOC` which checks if we ran OOM and if
so returns an error. Provide a `#nodef` for Coverity to mark the
error case as an abort path.
coverity: hint git_vector_foreach does not deref NULL contents
Coverity does not comprehend the connection between a vector's
size and the contents pointer, that is that the vector's pointer
is non-NULL when its size is positive. As the vector code should
be reasonably well tested and users are expected to not manually
modify a vector's contents it seems save to assume that the
macros will never dereference a NULL pointer.
Fix Coverity warnings by overriding the foreach macros with
macros that explicitly aborting when (v)->contents is NULL.
signature: use GITERR_CHECK_ALLOC to check for OOM situation
When checking for out of memory situations we usually use the
GITERR_CHECK_ALLOC macro. Besides conforming to our current code
base it adds the benefit of silencing errors in Coverity due to
Coverity handling the macro's error path as abort.
When checking if a string is prefixed by a drive letter (e.g.
"C:") we verify this by inspecting the first and second character
of the string. Coverity thinks this is a defect as we do not
check the string's length first, but in fact we only check the
second character if the first character is part of the alphabet,
that is it cannot be '\0'.
Fix this by overriding the macro and explicitly checking the
string's length.
Add nodefs for macros that abort the current flow due to errors.
This includes macros that trigger on integer overflows and for
the version check macro. This aids Coverity as we point out that
these paths will cause a fatal error.
Edward Thomson [Tue, 16 Feb 2016 17:11:46 +0000 (17:11 +0000)]
index: allow read of index w/ illegal entries
Allow `git_index_read` to handle reading existing indexes with
illegal entries. Allow the low-level `git_index_add` to add
properly formed `git_index_entry`s even if they contain paths
that would be illegal for the current filesystem (eg, `AUX`).
Continue to disallow `git_index_add_bypath` from adding entries
that are illegal universally illegal (eg, `.git`, `foo/../bar`).
Edward Thomson [Tue, 16 Feb 2016 13:08:55 +0000 (13:08 +0000)]
iterator: assert tree_iterator has a frame
Although a `tree_iterator` that failed to be properly created
does not have a frame, all other `tree_iterator`s should. Do not
call `pop` in the failure case, but assert that in all other
cases there is a frame.
Colin Xu [Fri, 22 Jan 2016 08:03:37 +0000 (16:03 +0800)]
Validate pointer before access the member.
When Git repository at network locations, sometimes git_iterator_for_tree
fails at iterator__update_ignore_case so it goes to git_iterator_free.
Null pointer will crash the process if not check.
Edward Thomson [Tue, 16 Feb 2016 18:50:08 +0000 (18:50 +0000)]
win32: tests around handling forbidden paths
Introduce a repository that contains some paths that were illegal
on PC-DOS circa 1981 (like `aux`, `con`, `com1`) and that in a
bizarre fit of retrocomputing, remain illegal on some "modern"
computers, despite being "new technology".
Introduce some aspirational tests that suggest that we should be
able to cope with trees and indexes that contain paths that
would be illegal on the filesystem, so that we can at least diff
them. Further ensure that checkout will not write a repository
with forbidden paths.
We should be checking whether the object we're looking up is a commit,
and we should let the caller know whether the not-found return code
comes from a bad object type or just a missing signature.
Edward Thomson [Mon, 15 Feb 2016 17:16:00 +0000 (17:16 +0000)]
rebase: persist a single in-memory index
When performing an in-memory rebase, keep a single index for the
duration, so that callers have the expected index lifecycle and
do not hold on to an index that is free'd out from under them.
When we moved the logic to handle the first one, wrong loop logic was
kept in place which meant we still finished early. But we now notice it
because we're not reading past the last LF we find.
This was not noticed before as the last field in the tested commit was
multi-line which does not trigger the early break.
Edward Thomson [Thu, 11 Feb 2016 18:48:48 +0000 (10:48 -0800)]
rebase: introduce inmemory rebasing
Introduce the ability to rebase in-memory or in a bare repository.
When `rebase_options.inmemory` is specified, the resultant `git_rebase`
session will not be persisted to disk. Callers may still analyze
the rebase operations, resolve any conflicts against the in-memory
index and create the commits. Neither `HEAD` nor the working
directory will be updated during this process.
When posting our instrumented build results to Coverity we have
to include sensitive information, in particular our authorization
token. Currently we use an unencrypted channel to post this
information, leading to the token being transferred in plain.
Coverity currently lists a lot of errors with regard to
GITERR_CHECK_ALLOC causing resource leaks. We know this macro is
only invoked when we want to abort because we are out of memory.
Coverity allows for overriding the default model where we know
that certain functions guarantee a desired behavior. The
user_nodefs.h is used to override the behavior of macros.
Re-define GITERR_CHECK_ALLOC inside of it to specify its abort
nature.
The function `git_packfile_stream_open` tries to free the passed
in stream when an error occurs. The only call site is
`git_indexer_append`, though, which passes in the address of a
stream struct which has not been allocated on the heap.
Fix the issue by simply removing the call to free. In case of an
error we did not allocate any memory yet and otherwise it should
be the caller's responsibility to manage it's object's lifetime.
Edward Thomson [Tue, 9 Feb 2016 01:55:22 +0000 (17:55 -0800)]
Better document `git_merge_commits` redux
`git_merge_commits` and `git_merge` now *do* handle recursive base
building for criss-cross merges. Remove the documentation that says
that they do not.
Edward Thomson [Sun, 7 Feb 2016 21:16:30 +0000 (13:16 -0800)]
filter: avoid races during filter registration
Previously we would set the global filter registry structure before
adding filters to the structure, without a lock, which is quite racy.
Now, register default filters during global registration and use an
rwlock to read and write the filter registry (as appopriate).
Edward Thomson [Tue, 19 Jan 2016 17:13:23 +0000 (11:13 -0600)]
winhttp: name mangle class / iid on mingw
Standard Windows type systems define CLSID_InternetSecurityManager
and IID_IInternetSecurityManager, but MinGW lacks these definitions.
As a result, we must hardcode these definitions ourselves. However,
we should not use a public struct with those names, lest another
library do the same thing and consumers cannot link to both.