Commit Graph

66564 Commits

Author SHA1 Message Date
Ævar Arnfjörð Bjarmason
a8cc594333 hooks: fix an obscure TOCTOU "did we just run a hook?" race
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>
2022-03-07 13:00:53 -08:00
Ævar Arnfjörð Bjarmason
9f6e63b966 merge: don't run post-hook logic on --no-verify
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>
2022-03-07 13:00:52 -08:00
Ævar Arnfjörð Bjarmason
99d60545f8 string-list API: change "nr" and "alloc" to "size_t"
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>
2022-03-07 12:02:04 -08:00
Ævar Arnfjörð Bjarmason
6f69325258 gettext API users: don't explicitly cast ngettext()'s "n"
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>
2022-03-07 11:57:52 -08:00
Derrick Stolee
6dbf4b8172 commit-graph: declare bankruptcy on GDAT chunks
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>
2022-03-07 09:17:03 -08:00
Junio C Hamano
c2162907e9 The ninth batch
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-06 21:25:33 -08:00
Junio C Hamano
7a4e06c42a Merge branch 'jt/ls-files-stage-recurse'
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
2022-03-06 21:25:33 -08:00
Junio C Hamano
11da0a5580 Merge branch 'gc/stash-on-branch-with-multi-level-name'
"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
2022-03-06 21:25:33 -08:00
Junio C Hamano
061fd5727d Merge branch 'ah/advice-switch-requires-detach-to-detach'
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
2022-03-06 21:25:32 -08:00
Junio C Hamano
20d34c07ea Merge branch 'ab/c99-designated-initializers'
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"
2022-03-06 21:25:32 -08:00
Junio C Hamano
283e4e7cd3 Merge branch 'mc/index-pack-report-max-size'
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
2022-03-06 21:25:32 -08:00
Junio C Hamano
6d8d81ec36 Merge branch 'ac/usage-string-fixups'
Usage-string normalization.

* ac/usage-string-fixups:
  amend remaining usage strings according to style guide
2022-03-06 21:25:32 -08:00
Junio C Hamano
a281069e77 Merge branch 'ab/test-leak-diag'
Random test-framework clean-up.

* ab/test-leak-diag:
  test-lib: add "fast_unwind_on_malloc=0" to LSAN_OPTIONS
  test-lib: make $GIT_BUILD_DIR an absolute path
  test-lib: correct and assert TEST_DIRECTORY overriding
  test-lib: add GIT_SAN_OPTIONS, inherit [AL]SAN_OPTIONS
2022-03-06 21:25:31 -08:00
Junio C Hamano
6878ea6f14 Merge branch 'ab/hook-tests'
Test modernization.

* ab/hook-tests:
  hook tests: use a modern style for "pre-push" tests
  hook tests: test for exact "pre-push" hook input
2022-03-06 21:25:31 -08:00
Junio C Hamano
ae59346f09 Merge branch 'en/merge-ort-plug-leaks'
Leakfix.

* en/merge-ort-plug-leaks:
  merge-ort: fix small memory leak in unique_path()
  merge-ort: fix small memory leak in detect_and_process_renames()
2022-03-06 21:25:31 -08:00
Junio C Hamano
aae90a156d Merge branch 'ds/worktree-docs'
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
2022-03-06 21:25:31 -08:00
Junio C Hamano
50e0dd8fee Merge branch 'jc/rerere-train-modernise'
Small modernization of the rerere-train script (in contrib/).

* jc/rerere-train-modernise:
  rerere-train: two fixes to the use of "git show -s"
2022-03-06 21:25:30 -08:00
Junio C Hamano
e828747001 Merge branch 'rs/bisect-executable-not-found'
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
2022-03-06 21:25:30 -08:00
Junio C Hamano
967176465a Merge branch 'en/sparse-checkout-fixes'
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
2022-03-06 21:25:30 -08:00
Junio C Hamano
b6c596fd01 Merge branch 'cg/t3903-modernize'
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_*
2022-03-06 21:25:30 -08:00
Glen Choo
c9d2562493 submodule--helper update-clone: check for --filter and --init
"git submodule update --filter" also requires the "--init" option. Teach
update-clone to do this usage check in C and remove the check from
git-submodule.sh.

In addition, change update-clone's usage string so that it teaches users
about "git submodule update" instead of "git submodule--helper
update-clone" (the string is copied from git-submodule.sh). This should
be more helpful to users since they don't invoke update-clone directly.

Signed-off-by: Glen Choo <chooglen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04 16:39:13 -08:00
Glen Choo
104744f91d submodule update: add tests for --filter
Test the "--filter" option to make sure we don't break anything while
refactoring "git submodule update".

Signed-off-by: Glen Choo <chooglen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04 16:39:12 -08:00
Glen Choo
97cb977c82 submodule--helper: remove ensure-core-worktree
Move the logic of "git submodule--helper ensure-core-worktree" into
run-update-procedure, and since this makes the ensure-core-worktree
command obsolete, remove it.

As a result, the order of two operations in git-submodule.sh is
reversed: 'set the value of core.worktree' now happens after the call to
"git submodule--helper relative-path". This is safe - "relative-path"
does not depend on the value of core.worktree.

Signed-off-by: Glen Choo <chooglen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04 16:39:12 -08:00
Glen Choo
29a5e9e1ff submodule--helper update-clone: learn --init
Teach "git submodule--helper update-clone" the --init flag and remove
the corresponding shell code.

When the `--init` flag is passed to the subcommand, we do not spawn a
new subprocess and call `submodule--helper init` on the submodule paths,
because the Git machinery is not able to pick up the configuration
changes introduced by that init call. So we instead run the
`init_submodule_cb()` callback over each submodule in the same process.

[1] https://lore.kernel.org/git/CAP8UFD0NCQ5w_3GtT_xHr35i7h8BuLX4UcHNY6VHPGREmDVObA@mail.gmail.com/

Signed-off-by: Glen Choo <chooglen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04 16:39:12 -08:00
Atharva Raykar
3ce52cba5b submodule--helper: allow setting superprefix for init_submodule()
We allow callers of the `init_submodule()` function to optionally
override the superprefix from the environment.

We need to enable this option because in our conversion of the update
command that will follow, the '--init' option will be handled through
this API. We will need to change the superprefix at that time to ensure
the display paths show correctly in the output messages.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Shourya Shukla <periperidip@gmail.com>
Signed-off-by: Atharva Raykar <raykar.ath@gmail.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04 16:39:12 -08:00
Atharva Raykar
5312a850b8 submodule--helper: refactor get_submodule_displaypath()
We create a function called `do_get_submodule_displaypath()` that
generates the display path required by several submodule functions, and
takes a custom superprefix parameter, instead of reading it from the
environment.

We then redefine the existing `get_submodule_displaypath()` function
as a call to this new function, where the superprefix is obtained from
the environment.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Shourya Shukla <periperidip@gmail.com>
Signed-off-by: Atharva Raykar <raykar.ath@gmail.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04 16:39:12 -08:00
Glen Choo
1012a5cbc3 submodule--helper run-update-procedure: learn --remote
Teach run-update-procedure to handle --remote instead of parsing
--remote in git-submodule.sh. As a result, "git submodule--helper
[print-default-remote|remote-branch]" have no more callers, so remove
them.

Signed-off-by: Glen Choo <chooglen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04 16:39:12 -08:00
Ævar Arnfjörð Bjarmason
ed9c84853e submodule--helper: don't use bitfield indirection for parse_options()
Do away with the indirection of local variables added in
c51f8f94e5 (submodule--helper: run update procedures from C,
2021-08-24).

These were only needed because in C you can't get a pointer to a
single bit, so we were using intermediate variables instead.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Glen Choo <chooglen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04 16:39:12 -08:00
Atharva Raykar
a77c3fcb5e submodule--helper: get remote names from any repository
`get_default_remote()` retrieves the name of a remote by resolving the
refs from of the current repository's ref store.

Thus in order to use it for retrieving the remote name of a submodule,
we have to start a new subprocess which runs from the submodule
directory.

Let's instead introduce a function called `repo_get_default_remote()`
which takes any repository object and retrieves the remote accordingly.

`get_default_remote()` is then defined as a call to
`repo_get_default_remote()` with 'the_repository' passed to it.

Now that we have `repo_get_default_remote()`, we no longer have to start
a subprocess that called `submodule--helper get-default-remote` from
within the submodule directory.

So let's make a function called `get_default_remote_submodule()` which
takes a submodule path, and returns the default remote for that
submodule, all within the same process.

We can now use this function to save an unnecessary subprocess spawn in
`sync_submodule()`, and also in a subsequent patch, which will require
this functionality.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Shourya Shukla <periperidip@gmail.com>
Helped-by: Glen Choo <chooglen@google.com>
Signed-off-by: Atharva Raykar <raykar.ath@gmail.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Glen Choo <chooglen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04 16:39:11 -08:00
Glen Choo
e441966596 submodule--helper run-update-procedure: remove --suboid
Teach run-update-procedure to determine the oid of the submodule's HEAD
instead of doing it in git-submodule.sh.

Signed-off-by: Glen Choo <chooglen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04 16:39:11 -08:00
Glen Choo
1a0b78c953 submodule--helper: reorganize code for sh to C conversion
Introduce a function, update_submodule2(), that will implement the
functionality of run-update-procedure and its surrounding shell code in
submodule.sh. This name is temporary; it will replace update_submodule()
when the sh to C conversion is complete.

Signed-off-by: Glen Choo <chooglen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04 16:39:11 -08:00
Glen Choo
f7bdb32918 submodule--helper: remove update-module-mode
This is dead code - it has not been used since c51f8f94e5
(submodule--helper: run update procedures from C, 2021-08-24).

Signed-off-by: Glen Choo <chooglen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04 16:39:11 -08:00
Ævar Arnfjörð Bjarmason
aca8568e2c submodule tests: test for init and update failure output
Amend some submodule tests to test for the failure output of "git
submodule [update|init]". The lack of such tests hid a regression in
an earlier version of a subsequent commit.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Glen Choo <chooglen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04 16:39:11 -08:00
Ævar Arnfjörð Bjarmason
759f340738 repository.c: free the "path cache" in repo_clear()
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>
2022-03-04 13:24:19 -08:00
Ævar Arnfjörð Bjarmason
2d102c2bca range-diff: plug memory leak in read_patches()
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>
2022-03-04 13:24:19 -08:00
Ævar Arnfjörð Bjarmason
4998e93fa6 range-diff: plug memory leak in common invocation
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>
2022-03-04 13:24:19 -08:00
Ævar Arnfjörð Bjarmason
ef3fe21448 lockfile API users: simplify and don't leak "path"
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>
2022-03-04 13:24:19 -08:00
Ævar Arnfjörð Bjarmason
51a94d8ffe commit-graph: stop fill_oids_from_packs() progress on error and free()
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>
2022-03-04 13:24:19 -08:00
Ævar Arnfjörð Bjarmason
4a0479086a commit-graph: fix memory leak in misused string_list API
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>
2022-03-04 13:24:18 -08:00
Ævar Arnfjörð Bjarmason
8f79015111 submodule--helper: fix trivial leak in module_add()
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>
2022-03-04 13:24:18 -08:00
Ævar Arnfjörð Bjarmason
0f0d118c65 transport: stop needlessly copying bundle header references
Amend the logic added in fddf2ebe38 (transport: teach all vtables to
allow fetch first, 2019-08-21) and save ourselves pointless work in
fetch_refs_from_bundle().

The fetch_refs_from_bundle() caller doesn't care about the "struct
ref *result" return value of get_refs_from_bundle(), and doesn't need
any of the work we were doing in looping over the
"data->header.references" in get_refs_from_bundle().

So this change saves us work, and also fixes a memory leak that we had
when called from fetch_refs_from_bundle(). The other caller of
get_refs_from_bundle() is the "get_refs_list" member we set up for the
"struct transport_vtable bundle_vtable". That caller does care about
the "struct ref *result" return value.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04 13:24:18 -08:00
Ævar Arnfjörð Bjarmason
bf67dd8d9a bundle: call strvec_clear() on allocated strvec
Fixing this small memory leak in cmd_bundle_create() gets
"t5607-clone-bundle.sh" closer to passing under SANITIZE=leak.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04 13:24:18 -08:00
Ævar Arnfjörð Bjarmason
b07fa8f1b2 remote-curl.c: free memory in cmd_main()
Plug a trivial memory leak in code added in a2d725b7bd (Use an
external program to implement fetching with curl, 2009-08-05).

To do this have the cmd_main() use a "goto cleanup" pattern, and to
return an error of 1 unless we can fall through to the http_cleanup()
at the end.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04 13:24:18 -08:00
Ævar Arnfjörð Bjarmason
a41e8e7467 urlmatch.c: add and use a *_release() function
Plug a memory leak in credential_apply_config() by adding and using a
new urlmatch_config_release() function. This just does a
string_list_clear() on the "vars" member.

This finished up work on normalizing the init/free pattern in this
API, started in 73ee449bbf (urlmatch.[ch]: add and use
URLMATCH_CONFIG_INIT, 2021-10-01).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04 13:24:18 -08:00
Ævar Arnfjörð Bjarmason
a18d66cefb diff.c: free "buf" in diff_words_flush()
Amend the freeing logic added in e6e045f803 (diff.c: buffer all
output if asked to, 2017-06-29) to free the containing "buf" in
addition to its members.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04 13:24:18 -08:00
Ævar Arnfjörð Bjarmason
e69fe2e460 merge-base: free() allocated "struct commit **" list
Fix a memory leak in 53eda89b2f (merge-base: teach "git merge-base"
to drive underlying merge_bases_many(), 2008-07-30) by calling free()
on the "struct commit **" list used by "git merge-base".

This gets e.g. "t6010-merge-base.sh" closer to passing under
SANITIZE=leak, it failed 8 tests before when compiled with that
option, and now fails only 5 tests.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04 13:24:17 -08:00
Ævar Arnfjörð Bjarmason
f2bcc69e7e index-pack: fix memory leaks
Fix various memory leaks in "git index-pack", due to how tightly
coupled this command is with the revision walking this doesn't make
any new tests pass.

But e.g. this now passes, and had several failures before, i.e. we
still have failures in tests 3, 5 etc., which are being skipped here.

    ./t5300-pack-object.sh --run=1-2,4,6-27,30-42

It is a bit odd that we'll free "opts.anomaly", since the "opts" is a
"struct pack_idx_option" declared in pack.h. In pack-write.c there's a
reset_pack_idx_option(), but it only wipes the contents, but doesn't
free() anything.

Doing this here in cmd_index_pack() is correct because while the
struct is declared in pack.h, this code in builtin/index-pack.c (in
read_v2_anomalous_offsets()) is what allocates the "opts.anomaly", so
we should also free it here.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04 13:24:17 -08:00
Elia Pinto
131b94a10a test-lib.sh: Use GLIBC_TUNABLES instead of MALLOC_CHECK_ on glibc >= 2.34
In glibc >= 2.34 MALLOC_CHECK_ and MALLOC_PERTURB_ environment
variables have been replaced by GLIBC_TUNABLES.  Also the new
glibc requires that you preload a library called libc_malloc_debug.so
to get these features.

Using the ordinary glibc system variable detect if this is glibc >= 2.34 and
use GLIBC_TUNABLES and the new library.

This patch was inspired by a Richard W.M. Jones ndbkit patch

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04 11:58:30 -08:00
Todd Zullinger
b0b70d54c4 t/lib-gpg: kill all gpg components, not just gpg-agent
The gpg-agent is one of several processes that newer releases of GnuPG
start automatically.  Issue a kill to each of them to ensure they do not
affect separate tests.  (Yes, the separate GNUPGHOME should do that
already. If we find that is case, we could drop the --kill entirely.)

In terms of compatibility, the 'all' keyword was added to the --kill &
--reload options in GnuPG 2.1.18.  Debian and RHEL are often used as
indicators of how a change might affect older systems we often try to
support.

    - Debian Strech (old old stable), which has limited security support
      until June 2022, has GnuPG 2.1.18 (or 2.2.x in backports).

    - CentOS/RHEL 7, which is supported until June 2024, has GnuPG
      2.0.22, which lacks the --kill option, so the change won't have
      any impact.

Signed-off-by: Todd Zullinger <tmz@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04 11:36:42 -08:00
Todd Zullinger
fa47dd6445 t/lib-gpg: reload gpg components after updating trustlist
With gpgsm from gnupg-2.3, the changes to the trustlist.txt do not
appear to be picked up without refreshing the gpg-agent.  Use the 'all'
keyword to reload all of the gpg components.  The scdaemon is started as
a child of gpg-agent, for example.

We used to have a --kill at this spot, but I removed it in 2e285e7803
(t/lib-gpg: drop redundant killing of gpg-agent, 2019-02-07).  It seems
like it might be necessary (again) for 2.3.

Signed-off-by: Todd Zullinger <tmz@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04 11:36:41 -08:00