Now that all consumers of traverse_commit_list_filtered() populate the
'filter' member of 'struct rev_info', we can drop that parameter from
the method prototype to simplify things. In addition, the only thing
different now between traverse_commit_list_filtered() and
traverse_commit_list() is the presence of the 'omitted' parameter, which
is only non-NULL for one caller. We can consolidate these two methods by
having one call the other and use the simpler form everywhere the
'omitted' parameter would be NULL.
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Now that all consumers of prepare_bitmap_walk() have populated the
'filter' member of 'struct rev_info', we can drop that extra parameter
from the method and access it directly from the 'struct rev_info'.
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In builtin/pack-objects.c, we use a 'filter_options' global to populate
the --filter=<X> argument. The previous change created a pointer to a
filter option in 'struct rev_info', so we can use that pointer here as a
start to simplifying some usage of object filters.
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Placing a 'struct list_objects_filter_options' within 'struct rev_info'
will assist making some bookkeeping around object filters in the future.
For now, let's use this new member to remove a static global instance of
the struct from builtin/rev-list.c.
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
As we add more embedded members with type 'struct
list_objects_filter_options', it will be important to easily perform a
deep copy across multiple such structs. Create
list_objects_filter_copy() to satisfy this need.
This method is recursive to match the recursive nature of the struct.
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The --promisor option of 'git index-pack' was created in 88e2f9e
(introduce fetch-object: fetch one promisor object, 2017-12-05) but was
untested. It is currently unused within the Git codebase, but that will
change in an upcoming change to 'git bundle unbundle' when there is a
filter capability.
For now, add documentation about the option and add a test to ensure it
is working as expected.
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Before this change, gitweb would generate pages which included:
<meta http-equiv="content-type" content="application/xhtml+xml; charset=utf-8"/>
When a meta's http-equiv equals "content-type", the http-equiv is said
to be in the "Encoding declaration state". According to the HTML
Standard,
The Encoding declaration state may be used in HTML documents,
but elements with an http-equiv attribute in that state must not
be used in XML documents.
Source: <https://html.spec.whatwg.org/multipage/semantics.html#attr-meta-http-equiv-content-type>
This change removes that meta element since gitweb always generates XML
documents.
Signed-off-by: Jason Yundt <jason@jasonyundt.email>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change a fragile test pattern introduced in 6534703059 (Topo-sort
before --simplify-merges, 2008-08-03) to check the exit code of both
"git name-rev" and "git log".
This test as a whole would fail under SANITIZE=leak, but we'd pass
several "failing" tests due to hiding these exit codes before we'd
spot git dying with abort(). Now we'll instead spot all of the
failures.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change a fragile pattern introduced in 696acf45f9 (checkout:
implement "-" abbreviation, add docs and tests, 2009-01-17) to check
the exit code of both "git symbolic-ref" and "git rev-parse".
Without this change this test will become flaky e.g. under
SANITIZE=leak if some (but not all) memory leaks revealed by these
commands are fixed.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix code added in 969c877506 (git apply --directory broken for new
files, 2008-10-12) so that it doesn't invoke "git ls-files" on the
left-hand-side of a pipe, instead let's use an intermediate file.
Since we're doing that we can also drop the sub-shell that was here to
group the two.
There are a lot of these sorts of patterns in the test suite, and
there's no particular reason to fix this one other than in a preceding
commit all similar patterns except this one were fixed in
"t/t4128-apply-root.sh", so let's fix this one straggler as well.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Amend a prerequisite check added in 5c1ebcca4d (grep/icase: avoid
kwsset on literal non-ascii strings, 2016-06-25) to do invoke
'test-tool regex' in such a way that we'll notice if it dies under
SANITIZE=leak due to having a memory leak, as opposed to us not having
the "ICASE" support we're checking for.
Because we weren't making a distinction between the two I'd marked
these tests as passing under SANITIZE=leak in 03d85e21951 (leak tests:
mark remaining leak-free tests as such, 2021-12-17).
Doing this is tricky. Ideally "test_lazy_prereq" would materialize as
a "real" test that we could check the exit code of with the same
signal matching that "test_must_fail" does.
However lazy prerequisites aren't real tests, and are instead lazily
materialized in the guts of "test_have_prereq" when we've already
started another test.
We could detect the abort() (or similar) there and pass that exit code
down, and fail the test that caused the prerequisites to be
materialized.
But that would require extensive changes to test-lib.sh and
test-lib-functions.sh. Let's instead simply check if the exit code of
"test-tool regex" is zero, and if so set the prerequisites. If it's
non-zero let's run it again with "test_must_fail". We'll thus make a
distinction between "bad" non-zero (segv etc) and "good" (exit 1 etc.).
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change a couple of uses of "test_expect_failure" to use a
"test_expect_success" to positively assert the current behavior, and
replace the intent of "test_expect_failure" with a "TODO" comment int
the description.
As noted in [1] the "test_expect_failure" feature is overly eager to
accept any failure as OK, and thus by design hides segfaults, abort()
etc. Because of that I didn't notice in dd9cede913 (leak tests: mark
some rev-list tests as passing with SANITIZE=leak, 2021-10-31) that
this test leaks memory under SANITIZE=leak.
I have some larger local changes to add a better
"test_expect_failure", which would work just like
"test_expect_success", but would allow us say "test_todo" here (and
"success" would emit a "not ok [...] # TODO", not "ok [...]".
So even though using "test_expect_success" here comes with its own
problems[2], let's use it as a narrow change to fix the problem at
hand here and stop conflating the current "success" with actual
SANITIZE=leak failures.
1. https://lore.kernel.org/git/87tuhmk19c.fsf@evledraar.gmail.com/
2. https://lore.kernel.org/git/xmqq4k9kj15p.fsf@gitster.g/
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change a fragile pattern introduced in 2b459b483c (diff: make sure
work tree side is shown as 0{40} when different, 2008-03-02) to check
the exit code of "git rev-list", while we're at it let's get rid of
the needless sub-shell for invoking it in favor of the "-C" option.
Because of this I'd marked these tests as passing under SANITIZE=leak
in 16d4bd4f14 (leak tests: mark some diff tests as passing with
SANITIZE=leak, 2021-10-31), let's remove the
"TEST_PASSES_SANITIZE_LEAK=true" annotation as they no longer do.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change a fragile test pattern that's been with us ever since these
tests were introduced in [1], [2] and [3] to properly return the exit
code of the failing command on failure.
Because of this I'd marked this test as passing under SANITIZE=leak in
[4] and [5]. We need to remove those annotations as these tests will
no longer pass.
1. 9081a421a6 (checkout: fix "branch info" memory leaks, 2021-11-16)
2. 0057c0917d (Add selftests verifying that we can parse notes trees
with various fanouts, 2009-10-09)
3. 048cdd4665 (t3305: Verify that adding many notes with git-notes
triggers increased fanout, 2010-02-13)
4. ca08972495 (leak tests: mark some notes tests as passing with
SANITIZE=leak, 2021-10-31)
5. 9081a421a6 (checkout: fix "branch info" memory leaks, 2021-11-16)
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Amend a test added in 9c46c054ae (rev-parse: tests git rev-parse
--verify master@{n}, for various n, 2010-08-24) so that we'll stop
ignoring the exit code of "git reflog" by having it on the
left-hand-side of a pipe.
Because of this I'd marked this test as passing under SANITIZE=leak in
f442c94638 (leak tests: mark some rev-parse tests as passing with
SANITIZE=leak, 2021-10-31). As all of it except this specific test
will now pass, let's skip it under the !SANITIZE_LEAK prerequisite.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
As in the preceding commit change a similar fragile test pattern
introduced in b798671fa9 (merge-recursive: do not rudely die on
binary merge, 2007-08-14) to use a "test_must_fail" instead.
Before this we wouldn't distinguish normal "git merge" failures from
segfaults or abort(). Unlike the preceding commit we didn't end up
hiding any SANITIZE=leak failures in this case, but let's
correspondingly change these anyway.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change a fragile test pattern introduced in 6b763c424e (git-apply: do
not read past the end of buffer, 2007-09-05). Before this we wouldn't
distinguish normal "git apply" failures from segfaults or abort().
I'd previously marked this test as passing under SANITIZE=leak in
f54f48fc07 (leak tests: mark some apply tests as passing with
SANITIZE=leak, 2021-10-31). Let's remove that annotation as this test
will no longer pass.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix a test pattern that originated in f1af60bdba (Support 'diff=pgm'
attribute, 2007-04-22) so that we'll stop using "git diff" on the
left-hand-side of a pipe, and thus ignoring its exit code.
Rather than use intermediate files let's rewrite these tests to a much
simpler but more exhaustive "test_tmp" where we'll ignore certain
fields in the output.
Note that this is not a faithful conversion of the previous
"read/test" in some cases, as we were ignoring more fields there than
we strictly needed to. Now we'll "test_cmp" everything we can, and
only ignore the likes of paths to $TEMPDIR etc.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix a test pattern that originated in f1af60bdba (Support 'diff=pgm'
attribute, 2007-04-22) so that we'll stop using "git diff" on the
left-hand-side of a pipe, and thus ignoring its exit code.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix an issue with the exit code of "diff-files" being ignored, which
has been ignored ever since these tests were originally added in
c859600954 ([PATCH] read-tree: save more user hassles during
fast-forward., 2005-06-07).
Since the exit code was ignored we'd hide errors here under
SANITIZE=leak, which resulted in me mistakenly marking these tests as
passing under SANITIZE=leak in e5a917fcf4 (unpack-trees: don't leak
memory in verify_clean_subdirectory(), 2021-10-07) and
4ea08416b8 (leak tests: mark a read-tree test as passing
SANITIZE=leak, 2021-10-31).
As it would be non-trivial to fix these tests (the leak is in
revision.c) let's un-mark them as passing under SANITIZE=leak in
addition to fixing the issue of ignoring the exit code.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Use the test_stdout_line_count helper added in
cdff1bb5a3 (test-lib-functions: introduce test_stdout_line_count,
2021-07-04) so that we'll spot if git itself dies, segfaults etc in
these expressions.
Because we didn't distinguish these failure conditions before I'd
mistakenly marked these tests as passing under SANITIZE=leak in
dd9cede913 (leak tests: mark some rev-list tests as passing with
SANITIZE=leak, 2021-10-31).
While we're at it let's re-indent these lines to match our usual
style, as we're having to change all of them anyway.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change some of the patterns in the test suite where we were hiding the
exit code from "git" by invoking it in a sub-shell within a "test"
expression to use temporary files and test_cmp instead.
These are not all the occurrences of this anti-pattern, but these in
particular hid issues where LSAN was dying, and I'd thus marked these
tests as passing under the linux-leaks CI job in past commits with
"TEST_PASSES_SANITIZE_LEAK=true". Let's deal with that by either
removing that marking, or skipping specific tests under
!SANITIZE_LEAK.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix a Time-of-check to time-of-use (TOCTOU) race in code added in
680ee550d7 (commit: skip discarding the index if there is no
pre-commit hook, 2017-08-14).
This obscure race condition can occur if we e.g. ran the "pre-commit"
hook and it modified the index, but hook_exists() returns false later
on (e.g., because the hook itself went away, the directory became
unreadable, etc.). Then we won't call discard_cache() when we should
have.
The race condition itself probably doesn't matter, and users would
have been unlikely to run into it in practice. This problem has been
noted on-list when 680ee550d7 was discussed[1], but had not been
fixed.
This change is mainly intended to improve the readability of the code
involved, and to make reasoning about it more straightforward. It
wasn't as obvious what we were trying to do here, but by having an
"invoked_hook" it's clearer that e.g. our discard_cache() is happening
because of the earlier hook execution.
Let's also change this for the push-to-checkout hook. Now instead of
checking if the hook exists and either doing a push to checkout or a
push to deploy we'll always attempt a push to checkout. If the hook
doesn't exist we'll fall back on push to deploy. The same behavior as
before, without the TOCTOU race. See 0855331941 (receive-pack:
support push-to-checkout hook, 2014-12-01) for the introduction of the
previous behavior.
This leaves uses of hook_exists() in two places that matter. The
"reference-transaction" check in refs.c, see 6754159767 (refs:
implement reference transaction hook, 2020-06-19), and the
"prepare-commit-msg" hook, see 66618a50f9 (sequencer: run
'prepare-commit-msg' hook, 2018-01-24).
In both of those cases we're saving ourselves CPU time by not
preparing data for the hook that we'll then do nothing with if we
don't have the hook. So using this "invoked_hook" pattern doesn't make
sense in those cases.
The "reference-transaction" and "prepare-commit-msg" hook also aren't
racy. In those cases we'll skip the hook runs if we race with a new
hook being added, whereas in the TOCTOU races being fixed here we were
incorrectly skipping the required post-hook logic.
1. https://lore.kernel.org/git/20170810191613.kpmhzg4seyxy3cpq@sigill.intra.peff.net/
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix a minor bug introduced in bc40ce4de6 (merge: --no-verify to
bypass pre-merge-commit hook, 2019-08-07), when that change made the
--no-verify option bypass the "pre-merge-commit" hook it didn't update
the corresponding find_hook() (later hook_exists()) condition.
As can be seen in the preceding commit in 6098817fd7 (git-merge:
honor pre-merge-commit hook, 2019-08-07) the two should go hand in
hand. There's no point in invoking discard_cache() here if the hook
couldn't have possibly updated the index.
It's buggy that we use "hook_exist()" here, and as discussed in the
subsequent commit it's subject to obscure race conditions that we're
about to fix, but for now this change is a strict improvement that
retains any caveats to do with the use of "hooks_exist()" as-is.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change the "nr" and "alloc" members of "struct string_list" to use
"size_t" instead of "nr". On some platforms the size of an "unsigned
int" will be smaller than a "size_t", e.g. a 32 bit unsigned v.s. 64
bit unsigned. As "struct string_list" is a generic API we use in a lot
of places this might cause overflows.
As one example: code in "refs.c" keeps track of the number of refs
with a "size_t", and auxiliary code in builtin/remote.c in
get_ref_states() appends those to a "struct string_list".
While we're at it split the "nr" and "alloc" in string-list.h across
two lines, which is the case for most such struct member
declarations (e.g. in "strbuf.h" and "strvec.h").
Changing e.g. "int i" to "size_t i" in run_and_feed_hook() isn't
strictly necessary, and there are a lot more cases where we'll use a
local "int", "unsigned int" etc. variable derived from the "nr" in the
"struct string_list". But in that case as well as
add_wrapped_shortlog_msg() in builtin/shortlog.c we need to adjust the
printf format referring to "nr" anyway, so let's also change the other
variables referring to it.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change a few stray users of the inline gettext.h Q_() function to stop
casting its "n" argument, the vast majority of the users of that
wrapper API use the implicit cast to "unsigned long".
The ngettext() function (which Q_() resolves to) takes an "unsigned
long int", and so does our Q_() wrapper for it, see 0c9ea33b90 (i18n:
add stub Q_() wrapper for ngettext, 2011-03-09). The function isn't
ours, but provided by e.g. GNU libintl.
This amends code added in added in 7171a0b0cf (index-pack: correct
"len" type in unpack_data(), 2016-07-13). The cast it added for the
printf format to die() was needed, but not the cast to Q_().
Likewise the casts in strbuf.c added in 8f354a1fae (l10n: localizable
upload progress messages, 2019-07-02) and for
builtin/merge-recursive.c in ccf7813139 (i18n: merge-recursive: mark
error messages for translation, 2016-09-15) weren't needed.
In the latter case the cast was copy/pasted from the argument to
warning() itself, added in b74d779bd9 (MinGW: Fix compiler warning in
merge-recursive, 2009-05-23). The cast for warning() is needed, but
not the one for ngettext()'s "n" argument.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The Generation Data (GDAT) and Generation Data Overflow (GDOV) chunks
store corrected commit date offsets, used for generation number v2.
Recent changes have demonstrated that previous versions of Git were
incorrectly parsing data from these chunks, but might have also been
writing them incorrectly.
I asserted [1] that the previous fixes were sufficient because the known
reasons for incorrectly writing generation number v2 data relied on
parsing the information incorrectly out of a commit-graph file, but the
previous versions of Git were not reading the generation number v2 data.
However, Patrick demonstrated [2] a case where in split commit-graphs
across an alternate boundary (and possibly some other special
conditions) it was possible to have a commit-graph that was generated by
a previous version of Git have incorrect generation number v2 data which
results in errors like the following:
commit-graph generation for commit <oid> is 1623273624 < 1623273710
[1] https://lore.kernel.org/git/f50e74f0-9ffa-f4f2-4663-269801495ed3@github.com/
[2] https://lore.kernel.org/git/Yh93vOkt2DkrGPh2@ncase/
Clearly, there is something else going on. The situation is not
completely understood, but the errors do not reproduce if the
commit-graphs are all generated by a Git version including these recent
fixes.
If we cannot trust the existing data in the GDAT and GDOV chunks, then
we can alter the format to change the chunk IDs for these chunks. This
causes the new version of Git to silently ignore the older chunks (and
disabling generation number v2 in the process) while writing new
commit-graph files with correct data in the GDA2 and GDO2 chunks.
Update commit-graph-format.txt including a historical note about these
deprecated chunks.
Reported-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Many output modes of "ls-files" do not work with its
"--recurse-submodules" option, but the "-s" mode has been taught to
work with it.
* jt/ls-files-stage-recurse:
ls-files: support --recurse-submodules --stage
"git checkout -b branch/with/multi/level/name && git stash" only
recorded the last level component of the branch name, which has
been corrected.
* gc/stash-on-branch-with-multi-level-name:
stash: strip "refs/heads/" with skip_prefix
The error message given by "git switch HEAD~4" has been clarified
to suggest the "--detach" option that is required.
* ah/advice-switch-requires-detach-to-detach:
switch: mention the --detach option when dying due to lack of a branch
Use designated initializers we started using in mid 2017 in more
parts of the codebase that are relatively quiescent.
* ab/c99-designated-initializers:
fast-import.c: use designated initializers for "partial" struct assignments
refspec.c: use designated initializers for "struct refspec_item"
convert.c: use designated initializers for "struct stream_filter*"
userdiff.c: use designated initializers for "struct userdiff_driver"
archive-*.c: use designated initializers for "struct archiver"
object-file: use designated initializers for "struct git_hash_algo"
trace2: use designated initializers for "struct tr2_dst"
trace2: use designated initializers for "struct tr2_tgt"
imap-send.c: use designated initializers for "struct imap_server_conf"
When "index-pack" dies due to incoming data exceeding the maximum
allowed input size, include the value of the limit in the error
message.
* mc/index-pack-report-max-size:
index-pack: clarify the breached limit
Tighten the language around "working tree" and "worktree" in the
docs.
* ds/worktree-docs:
worktree: use 'worktree' over 'working tree'
worktree: use 'worktree' over 'working tree'
worktree: use 'worktree' over 'working tree'
worktree: use 'worktree' over 'working tree'
worktree: use 'worktree' over 'working tree'
worktree: use 'worktree' over 'working tree'
worktree: use 'worktree' over 'working tree'
worktree: extract checkout_worktree()
worktree: extract copy_sparse_checkout()
worktree: extract copy_filtered_worktree_config()
worktree: combine two translatable messages
A not-so-common mistake is to write a script to feed "git bisect
run" without making it executable, in which case all tests will
exit with 126 or 127 error codes, even on revisions that are marked
as good. Try to recognize this situation and stop iteration early.
* rs/bisect-executable-not-found:
bisect--helper: double-check run command on exit code 126 and 127
bisect: document run behavior with exit codes 126 and 127
bisect--helper: release strbuf and strvec on run error
bisect--helper: report actual bisect_state() argument on error
Further polishing of "git sparse-checkout".
* en/sparse-checkout-fixes:
sparse-checkout: reject arguments in cone-mode that look like patterns
sparse-checkout: error or warn when given individual files
sparse-checkout: pay attention to prefix for {set, add}
sparse-checkout: correctly set non-cone mode when expected
sparse-checkout: correct reapply's handling of options
Test modernization.
* cg/t3903-modernize:
tests: make the code more readable
tests: allow testing if a path is truly a file or a directory
t/t3903-stash.sh: replace test [-d|-f] with test_path_is_*
The "struct path_cache" added in 102de880d2 (path.c: migrate global
git_path_* to take a repository argument, 2018-05-17) is only used
directly by code in repository.[ch] (but populated in path.[ch]).
Let's move this code to repository.[ch], and stop leaking this memory
when we run repo_clear(). To avoid the cast change it from a "const
char *" to a "char *".
This also removes the "PATH_CACHE_INIT" macro, which has never been
used for anything. For the "struct repository" we already make a hard
assumption that it (and "the_repository") can be identically
initialized by making it a "static" variable, so making use of a
"PATH_CACHE_INIT" somewhere would have been confusing.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Amend code added in d9c66f0b5b (range-diff: first rudimentary
implementation, 2018-08-13) to use a "goto cleanup" pattern. This
makes for less code, and frees memory that we'd previously leak.
The reason for changing free(util) to FREE_AND_NULL(util) is because
at the end of the function we append the contents of "util" to a
"struct string_list" if it's non-NULL.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Create a public release_patch() version of the private free_patch()
function added in 13b5af22f3 (apply: move libified code from
builtin/apply.c to apply.{c,h}, 2016-04-22). Unlike the existing
function this one doesn't free() the "struct patch" itself, so we can
use it for variables on the stack.
Use it in range-diff.c to fix a memory leak in common range-diff
invocations, e.g.:
git -P range-diff origin/master origin/next origin/seen
Would emit several errors when compiled with SANITIZE=leak, but now
runs cleanly.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix a memory leak in code added in 6c622f9f0b (commit-graph: write
commit-graph chains, 2019-06-18). We needed to free the "lock_name" if
we encounter errors, and the "graph_name" after we'd run unlink() on
it.
For the case of write_commit_graph_file() refactoring the code to free
the "lock_name" after we were done using the "struct lock_file lk"
would have made the control flow more complex. Luckily we can free the
"lock_file" right after the hold_lock_file_for_update() call, if it
makes use of "path" at all it'll have copied its contents to a "struct
strbuf" of its own.
While I'm at it let's fix code added in fb10ca5b54 (sparse-checkout:
write using lockfile, 2019-11-21) in write_patterns_and_update() to
avoid the same complexity that I thought I needed when I wrote the
initial fix for write_commit_graph_file(). We can free the
"sparse_filename" right after calling hold_lock_file_for_update(), we
don't need to wait until we're exiting the function to do so.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix a bug in fill_oids_from_packs(), we should always stop_progress(),
but did not do so if we returned an error here. This also plugs a
memory leak in those cases by releasing the two "struct strbuf"
variables the function uses.
While I'm at it stop hardcoding "-1" here and just use the return
value of error() instead, which happens to be "-1".
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When this code was migrated to the string_list API in
d88b14b3fd (commit-graph: use string-list API for input, 2018-06-27)
it was made to use used both STRING_LIST_INIT_NODUP and a
strbuf_detach() pattern.
Those should not be used together if string_list_clear() is expected
to free the memory, instead we need to either use STRING_LIST_INIT_DUP
with a string_list_append_nodup(), or a STRING_LIST_INIT_NODUP and
manually fiddle with the "strdup_strings" member before calling
string_list_clear(). Let's do the former.
Since "strdup_strings = 1" is set now other code might be broken by
relying on "pack_indexes" not to duplicate it strings, but that
doesn't happen. When we pass this down to write_commit_graph() that
code uses the "struct string_list" without modifying it. Let's add a
"const" to the variable to have the compiler enforce that assumption.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix a memory leak in code added in a6226fd772 (submodule--helper:
convert the bulk of cmd_add() to C, 2021-08-10). If "realrepo" isn't a
copy of the "repo" member we should free() it.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
