diff_file: Fix crash when freeing a patch representing an empty untracked file #6475
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes a crash and a failed assert in the following scenario:
git_diff_index_to_workdir
;git_patch_from_diff
;Currently, this use case raises two issues:
git_diff_file_content__unload
may attempt to freegit_str__initstr
(a statically-allocated array)GIT_MMAP_VALIDATE
may failThe outcome depends on whether or not filters (such as “core.crlf”) were installed:
With filters,
fc->map.data
ultimately points togit_str__initstr
because the file is empty (convert_buf doesn’t touch “out”). However,GIT_DIFF_FLAG__FREE_DATA
is set; so, when time comes to free the patch,git_diff_file_content__unload
attempts to freegit_str__initstr
, resulting in a crash.Without any filters,
diff_file_content_load_workdir_file
attempts to mmap a 0-length file, causing GIT_MMAP_VALIDATE to fail.The fix I’m suggesting here is: in
diff_file_content_load_workdir_file
, if we know the file is empty, then don’t bother with mmap or readbuffer.I’m not the most familiar with libgit2’s codebase so feel free to edit the PR if there’s a better solution!
—
PS: Incidentally, this PR fixes 2 other tests when assertions are enabled:
test_diff_rename__empty_files_renamed
test_diff_workdir__can_diff_empty_file