POSIX emulation retries should be configurable so that tests can disable
them. In particular, maniacally threading tests may end up trying to
open locked files and need retries, which will slow continuous
integration tests significantly.
This can prevent FILE_SHARED_VIOLATIONS when used in tools such as TortoiseGit TGitCache and FILE_SHARE_DELETE, because files can be opened w/o being locked any more.
Signed-off-by: Sven Strickroth <email@cs-ware.de>
Provide a macro that will allow us to run a function with posix-like
return values multiple times in a retry loop, with an optional cleanup
function called between invocations.
Introduce mapping from windows error codes to errno values. This
allows us to replace our calls to the Windows posix emulation functions
with calls to the Win32 APIs for more fine-grained control over the
emulation.
These mappings match the Windows CRT's mappings for its posix emulation
as they were described to me.
While writing the tree inside of a buffer, we check whether the buffer
runs out of memory after each tree entry. While we set the error code as
soon as we detect the OOM situation, we happily proceed iterating over
the entries. This is not useful at all, as we will try to write into the
buffer repeatedly, which cannot work.
Fix this by exiting as soon as we are OOM.
The `git_tree_entry *entry` variable is defined twice inside of this
function. While this is not a problem currently, remove the shadowing
variable to avoid future confusion.
While we detect errors in `git_treebuilder_write_with_buffer`, we just
exit directly instead of freeing allocated memory. Fix this by
remembering error codes and skipping forward to the function's cleanup
code.
The recent addition of an error code to `pass_whole_blame` in ff8d2eb15
(blame_git: check return value of object lookup, 2017-03-20) introduced
a spurious goto. Remove it.
`O_EXCL` and `O_TRUNC` are mutually exclusive flags to open(2); you can't
truncate a file if you're asserting that it can't exist in the first place.
Drop `O_TRUNC`.
While parsing patch header lines, we iterate over each line and check if
the line has trailing garbage. What we do not check though is that the
line is actually a line ending with a trailing newline.
Fix this by checking the return code of `parse_advance_expected_str`.
The `pack_entry_find_prefix` function receives a `git_rawobj` structure
as argument. While the function first initializes the structure to a
sensible state, Coverity is unable to correctly detect this, resulting
in a warning.
Fix this warning by initializing the object to all-zeroes before passing
it to the function.
While parsing section headers, we use a buffer to store the actual
section name. We do not check though if the buffer runs out of memory at
any stage. Do so.
The function `pass_whole_blame` performs an object lookup but does not
check if the lookup actually succeeds. Convert the function to return an
error code and check for it in the calling function.
The OpenSSL library may require multiple locks to work correctly, where
it is the caller's responsibility to initialize and release the locks.
While we correctly initialized up to `n` locks, as determined by
`CRYPTO_num_locks`, we were repeatedly freeing the same mutex in our
shutdown procedure.
Fix the issue by freeing locks at the correct index.
The `map_free` functions were not implemented as functions but instead
as macros which also set the map to NULL. While this is most certainly
sensible in most cases, we should prefer the more obvious behavior,
namingly leaving the map pointer intact.
Furthermore, this macro has been refactored incorrectly during the
map-refactorings: the two statements are not actually grouped together
by a `do { ... } while (0)` block, as it is required for macros to
match the behavior of functions more closely. This has led to at least
one subtle nesting error in `pack-objects.c`. The following code block
```
if (pb->object_ix)
git_oidmap_free(pb->object_ix);
```
would be expanded to
```
if (pb->object_ix)
git_oidmap__free(pb->object_ix); pb->object_ix = NULL;
```
which is not what one woudl expect. While it is not a bug here as it
would simply become a no-op, the wrong implementation could lead to bugs
in other occasions.
Fix this by simply removing the macro altogether and replacing it with
real function calls. This leaves the burden of setting the pointer to
NULL afterwards to the caller, but this is actually expected and behaves
like other `free` functions.
We currently call `git_strmap_free` on `checkout_data.mkdir_map` in the
`checkout_data_clear` function. The only thing protecting us from a
double-free is that the `git_strmap_free` function is in fact not a
function, but a macro that also sets the map to NULL.
Remove the second call to `git_strmap_free` and explicitly set the map
member to NULL.
