Commit Graph

75205 Commits

Author SHA1 Message Date
7291699928 ref-filter: avoid extra copies of payload/signature
When we know we're going to show the subject or body of a tag or commit,
we call find_subpos(), which returns pointers and lengths for the three
parts: subject, body, signature.

Oddly, the function finds the signature twice: once by calling
parse_signature() at the start, which copies the signature into a
separate strbuf, and then again by calling parse_signed_buffer() after
we've parsed past the subject.

This is due to 482c119186 (gpg-interface: improve interface for parsing
tags, 2021-02-11) and 88bce0e24c (ref-filter: hoist signature parsing,
2021-02-11). The idea is that in a multi-hash world, tag signatures may
appear in the header, rather than at the end of the body, in which case
we need to extract them into a separate buffer.

But parse_signature() would never find such a buffer! It only looks for
signature lines (like "-----BEGIN PGP") at the start of each line,
without any header keyword. So this code will never find anything except
the usual in-body signature.

And the extra code has two downsides:

  1. We spend time copying the payload and signature into strbufs. That
     might even be useful if we ended up with a NUL-terminated copy of
     the payload data, but we throw it away immediately. And the
     signature, since it comes at the end of the message, is already its
     own NUL-terminated buffer.

     The overhead isn't huge, but I measured a pretty consistent 1-2%
     speedup running "git for-each-ref --format='%(subject)'" with this
     patch on a clone of linux.git.

  2. The output of find_subpos() is a set of three ptr/len combinations,
     but only two of them point into the original buffer. This makes the
     interface confusing: you can't do pointer comparisons between them,
     and you have to remember to free the signature buffer. Since
     there's only one caller, it's not too bad in practice, but it did
     bite me while working on the next patch (and simplifying it will
     pave the way for that).

In the long run we might have to go back to something like this
approach, if we do have multi-hash header signatures. But I would argue
that the extra buffer should kick in only for a header signature, and be
passed out of find_subpos() separately.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-09 16:26:09 -07:00
87fbddd57e t6300: drop newline from wrapped test title
We don't usually include newlines in test titles, because you get funny
TAP output like:

  ok 417 - show good signature with custom format
  ok 418 - show good signature with custom format
  			    with ssh
  ok 419 - signature atom with grade option and bad signature

where a TAP parser would ignore the extra line anyway, giving the wrong
title. This comes from 26c9c03f0a (ref-filter: add new "signature" atom,
2023-06-04), and I think it was probably just editor line wrapping.

I checked for other cases with:

  git grep "test_expect_success [A-Z_,]* '[^']*$"
  git grep 'test_expect_success [A-Z_,]* "[^"]*$'

but this was the only hit.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-09 16:26:09 -07:00
90f2c7240c ci: remove 'Upload failed tests' directories' step from linux32 jobs
Linux32 jobs seem to be getting:

    Error: This request has been automatically failed because it uses a
    deprecated version of `actions/upload-artifact: v1`. Learn more:
    https://github.blog/changelog/2024-02-13-deprecation-notice-v1-and-v2-of-the-artifact-actions/

before doing anything useful.  For now, disable the step.

Ever since actions/upload-artifact@v1 got disabled, mentioning the
offending version of it seems to stop anything from happening.  At
least this should run the same build and test.

See

    https://github.com/git/git/actions/runs/10780030750/job/29894867249

for example.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-09 16:00:53 -07:00
d70600526e Merge branch 'cp/unit-test-reftable-stack' into ps/reftable-exclude
* cp/unit-test-reftable-stack:
  t-reftable-stack: add test for stack iterators
  t-reftable-stack: add test for non-default compaction factor
  t-reftable-stack: use reftable_ref_record_equal() to compare ref records
  t-reftable-stack: use Git's tempfile API instead of mkstemp()
  t: harmonize t-reftable-stack.c with coding guidelines
  t: move reftable/stack_test.c to the unit testing framework
2024-09-09 10:13:44 -07:00
2b14ced370 t-reftable-stack: add test for stack iterators
reftable_stack_init_ref_iterator and reftable_stack_init_log_iterator
as defined by reftable/stack.{c,h} initialize a stack iterator to
iterate over the ref and log records in a reftable stack respectively.
Since these functions are not exercised by any of the existing tests,
add a test for them.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-09 10:12:56 -07:00
e87952443a t-reftable-stack: add test for non-default compaction factor
In a recent codebase update (commit ae8e378430, merge branch
'ps/reftable-write-options', 2024/05/13) the geometric factor used
in auto-compaction of reftable tables was made configurable. Add
a test to verify the functionality introduced by this update.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-09 10:12:56 -07:00
1052280136 t-reftable-stack: use reftable_ref_record_equal() to compare ref records
In the current stack tests, ref records are compared for equality
by sometimes using the dedicated function for ref-record comparison,
reftable_ref_record_equal(), and sometimes by explicity comparing
contents of the ref records.

The latter method is undesired because there can exist unequal ref
records with some of the contents being equal. Replace the latter
instances of ref-record comparison with the former. This has the
added benefit of preserving uniformity throughout the test file.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-09 10:12:15 -07:00
57f583c748 apply: support --ours, --theirs, and --union for three-way merges
--ours, --theirs, and --union are already supported in `git merge-file`
for automatically resolving conflicts in favor of one version or the
other, instead of leaving conflict markers in the file. Support them in
`git apply -3` as well because the two commands do the same kind of
file-level merges.

In case in the future --ours, --theirs, and --union gain a meaning
outside of three-way-merges, they do not imply --3way but rather must be
specified alongside it.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-09 10:07:24 -07:00
9a36ea37ae doc: remote.*.skip{DefaultUpdate,FetchAll} stops prefetch
Back when 7cc91a2f (Add the configuration option skipFetchAll,
2009-11-09) added for the sole purpose of adding skipFetchAll as a
synonym to skipDefaultUpdate, there was no explanation about the
reason why it was needed., but these two configuration variables
mean exactly the same thing.

Also, when we taught the "prefetch" task to "git maintenance" later,
we did make it pay attention to the setting, but we forgot to
document it.

Document these variables as synonyms that collectively implements
the last-one-wins semantics, and also clarify that the prefetch task
is also controlled by this variable.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-09 10:06:13 -07:00
39ba986b0e config.mak.uname: add HAVE_DEV_TTY to cygwin config section
If neither HAVE_DEV_TTY nor GIT_WINDOWS_NATIVE is set, while compiling
the 'compat/terminal.c' code, then the fallback code calls the system
getpass() function. Unfortunately, this ignores the 'echo' parameter of
the git_terminal_prompt() function, since it has no way to implement that
functionality. This results in a less than optimal user experience on
cygwin, which does not define either of those build flags.

However, cygwin does have a functional '/dev/tty', so that it can build
with HAVE_DEV_TTY and benefit from the improved user experience.

The improved git_terminal_prompt() function that comes with HAVE_DEV_TTY
is used in the git_prompt() function, which in turn is used by the
'git credential', 'git bisect' and 'git help' commands. In addition to
git_terminal_prompt(), read_key_without_echo() is likewise improved and
used by the 'git add -p' command.

While using the 'git credential fill' command, for example:

  $ printf "%s\n" protocol=https host=example.com path=git | ./git credential fill
  Username for 'https://example.com': user
  Password for 'https://user@example.com':
  protocol=https
  host=example.com
  username=user
  password=pass
  $

The 'user' name is now echoed while typing (the password isn't), where this
wasn't the case before.

When using the auto-correct feature:

  $ ./git -c help.autocorrect=prompt fred
  WARNING: You called a Git command named 'fred', which does not exist.
  Run 'grep' instead [y/N]? n
  $ ./git -c help.autocorrect=prompt fred
  WARNING: You called a Git command named 'fred', which does not exist.
  Run 'grep' instead [y/N]? y
  fatal: no pattern given
  $

The user can actually see what they are typing at the prompt. Similar
comments apply to 'git bisect':

  $ ./git bisect bad master~1
  You need to start by "git bisect start"

  Do you want me to do it for you [Y/n]? y
  status: waiting for both good and bad commits
  status: waiting for good commit(s), bad commit known
  $ ./git bisect reset
  Already on 'master-tmp'
  $

  $ ./git bisect start
  status: waiting for both good and bad commits
  $ ./git bisect bad master~1
  status: waiting for good commit(s), bad commit known
  $ ./git bisect next
  warning: bisecting only with a bad commit
  Are you sure [Y/n]? n
  $ ./git bisect reset
  Already on 'master-tmp'
  $

The read_key_without_echo() function leads to a much improved 'git add -p'
command, when the 'interactive.singleKey' configuration is set:

  $ cd ..
  $ mkdir test-git
  $ cd test-git
  $ git init -q
  $ echo foo >file
  $ git add file
  $ echo bar >file
  $ ../git/git -c interactive.singleKey=true add -p
  diff --git a/file b/file
  index 257cc56..5716ca5 100644
  --- a/file
  +++ b/file
  @@ -1 +1 @@
  -foo
  +bar
  (1/1) Stage this hunk [y,n,q,a,d,e,p,?]? y

  $

Note that, not only is the user input echoed, but that it is immediately
accepted (without having to type <return>) and the program exits with the
hunk staged (in this case) or not.

In order to reap these benefits, set the HAVE_DEV_TTY build flag in the
cygwin configuration section of config.mak.uname.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-08 21:57:59 -07:00
476abc39ba t-reftable-stack: use Git's tempfile API instead of mkstemp()
Git's tempfile API defined by $GIT_DIR/tempfile.{c,h} provides
a unified interface for tempfile operations. Since reftable/stack.c
uses this API for all its tempfile needs instead of raw functions
like mkstemp(), make the ported stack test strictly use Git's
tempfile API as well.

A bigger benefit is the fact that we know to clean up the tempfile
in case the test fails because it gets registered and pruned via a
signal handler.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-08 13:24:03 -07:00
e4e384f68d t: harmonize t-reftable-stack.c with coding guidelines
Harmonize the newly ported test unit-tests/t-reftable-stack.c
with the following guidelines:
- Single line 'for' statements must omit curly braces.
- Structs must be 0-initialized with '= { 0 }' instead of '= { NULL }'.
- Array sizes and indices should preferably be of type 'size_t' and
  not 'int'.
- Function pointers should be passed as 'func' and not '&func'.

While at it, remove initialization for those variables that are
re-used multiple times, like loop variables.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-08 13:24:03 -07:00
15e29ea1c6 t: move reftable/stack_test.c to the unit testing framework
reftable/stack_test.c exercises the functions defined in
reftable/stack.{c, h}. Migrate reftable/stack_test.c to the
unit testing framework. Migration involves refactoring the tests
to use the unit testing framework instead of reftable's test
framework and renaming the tests to be in-line with unit-tests'
standards.

Since some of the tests use set_test_hash() defined by
reftable/test_framework.{c, h} but these files are not
'#included' in the test file, copy this function in the
ported test file.

With the migration of stack test to the unit-tests framework,
"test-tool reftable" becomes a no-op. Hence, get rid of everything
that uses "test-tool reftable" alongside everything that is used
to implement it.

While at it, alphabetically sort the cmds[] list in
helper/test-tool.c by moving the entry for "dump-reftable".

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-08 13:24:03 -07:00
11591850dd diff: report dirty submodules as changes in builtin_diff()
The diff machinery has two ways to detect changes to set the exit code:
Just comparing hashes and comparing blob contents.  The latter is needed
if certain changes have to be ignored, e.g. with --ignore-space-change
or --ignore-matching-lines.  It's enabled by the diff_options flag
diff_from_contents.

The slower mode as never considered submodules (and subrepos) as changes
with --submodule=diff or --submodule=log, which is inconsistent with
--submodule=short (the default).  Fix it.

d7b97b7185 (diff: let external diffs report that changes are
uninteresting, 2024-06-09) set diff_from_contents if external diff
programs are allowed.  This is the default e.g. for git diff, and so
that change exposed the inconsistency much more widely.

Reported-by: David Hull <david.hull@friendbuy.com>
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-08 13:21:24 -07:00
87cf96094a diff: report copies and renames as changes in run_diff_cmd()
The diff machinery has two ways to detect changes to set the exit code:
Just comparing hashes and comparing blob contents.  The latter is needed
if certain changes have to be ignored, e.g. with --ignore-space-change
or --ignore-matching-lines.  It's enabled by the diff_options flag
diff_from_contents.

The slower mode has never considered copies and renames to be changes,
which is inconsistent with the quicker one.  Fix it.  Even if we ignore
the file contents (because it's empty or contains only ignored lines),
there's still the meta data change of adding or changing a filename, so
we need to report it in the exit code.

d7b97b7185 (diff: let external diffs report that changes are
uninteresting, 2024-06-09) set diff_from_contents if external diff
programs are allowed.  This is the default e.g. for git diff, and so
that change exposed the inconsistency much more widely.

Reported-by: Jorge Luis Martinez Gomez <jol@jol.dev>
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-08 13:21:23 -07:00
fb2b9815a4 advice: recommend GIT_ADVICE=0 for tools
The GIT_ADVICE environment variable was added implicitly in b79deeb554
(advice: add --no-advice global option, 2024-05-03) but was not
documented. Add documentation to show that it is an option for tools
that want to disable these messages. Make note that while the
--no-advice option exists, older Git versions will fail to parse that
option. The environment variable presents a way to change the behavior
of Git versions that understand it without disrupting older versions.

Co-authored-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-06 14:15:16 -07:00
ce31b82ca9 scalar: add --no-tags option to 'scalar clone'
Some large repositories use tags to track a huge list of release
versions. While this choice is costly on the ref advertisement, it is
further wasteful for clients who do not need those tags. Allow clients
to optionally skip the tag advertisement.

This behavior is similar to that of 'git clone --no-tags' implemented in
0dab2468ee (clone: add a --no-tags option to clone without tags,
2017-04-26), including the modification of the remote.origin.tagOpt
config value to include "--no-tags".

One thing that is opposite of the 'git clone' implementation is that
this allows '--tags' as an assumed option, which can be naturally negated
with '--no-tags'. The clone command does not accept '--tags' but allows
"--no-no-tags" as the negation of its '--no-tags' option.

While testing this option, combine the test with the previously untested
'--no-src' option introduced in 4527db8ff8 (scalar: add --[no-]src
option, 2023-08-28).

Signed-off-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-06 14:13:48 -07:00
4c42d5ff28 The thirteenth batch
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-06 10:38:52 -07:00
f1160b2700 Merge branch 'jk/maybe-unused-cleanup'
Code clean-up.

* jk/maybe-unused-cleanup:
  grep: prefer UNUSED to MAYBE_UNUSED for pcre allocators
  gc: drop MAYBE_UNUSED annotation from used parameter
2024-09-06 10:38:52 -07:00
21c66081ca Merge branch 'jc/unused-on-windows'
Fix more fallouts from -Werror=unused-parameter.

* jc/unused-on-windows:
  refs/files-backend: work around -Wunused-parameter
2024-09-06 10:38:51 -07:00
5ecd5fa58b Merge branch 'jk/unused-parameters'
Make our codebase compilable with the -Werror=unused-parameter
option.

* jk/unused-parameters:
  CodingGuidelines: mention -Wunused-parameter and UNUSED
  config.mak.dev: enable -Wunused-parameter by default
  compat: mark unused parameters in win32/mingw functions
  compat: disable -Wunused-parameter in win32/headless.c
  compat: disable -Wunused-parameter in 3rd-party code
  t-reftable-readwrite: mark unused parameter in callback function
  gc: mark unused config parameter in virtual functions
2024-09-06 10:38:50 -07:00
4476304a06 Merge branch 'jc/maybe-unused'
Developer doc updates.

* jc/maybe-unused:
  CodingGuidelines: also mention MAYBE_UNUSED
2024-09-06 10:38:50 -07:00
6dcb2db0fa Merge branch 'jk/send-email-mailmap'
"git send-email" learned "--mailmap" option to allow rewriting the
recipient addresses.

* jk/send-email-mailmap:
  send-email: add mailmap support via sendemail.mailmap and --mailmap
  check-mailmap: add options for additional mailmap sources
  check-mailmap: accept "user@host" contacts
2024-09-06 10:38:49 -07:00
66710f91ff .mailmap document current address.
Cox Communications no longer supports email and transfered accounts to
yahoo. I closed the account at yahoo since I use gmail.com.

Signed-off-by: Stephen P. Smith <ishchis2@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-06 09:31:15 -07:00
c02414a997 interpret-trailers: handle message without trailing newline
When git-interpret-trailers is used to add a trailer to a message that
does not end in a trailing newline, the new trailer is added on the line
immediately following the message instead of as a trailer block
separated from the message by a blank line.

For example, if a message's text was exactly "The subject" with no
trailing newline present, `git interpret-trailers --trailer
my-trailer=true` will result in the following malformed commit message:

    The subject
    my-trailer: true

While it is generally expected that a commit message should end with a
newline character, git-interpret-trailers should not be returning an
invalid message in this case.

Use `strbuf_complete_line` to ensure that the message ends with a
newline character when reading the input.

Signed-off-by: Brian Lyles <brianmlyles@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-06 09:21:44 -07:00
a71c47825d sparse-checkout: use fdopen_lock_file() instead of xfdopen()
When updating sparse patterns, we open a lock_file to write out the new
data. The lock_file struct holds the file descriptor, but we call
fdopen() to get a stdio handle to do the actual write.

After we finish writing, we fflush() so that all of the data is on disk,
and then call commit_lock_file() which closes the descriptor. But we
never fclose() the stdio handle, leaking it.

The obvious solution seems like it would be to just call fclose(). But
when? If we do it before commit_lock_file(), then the lock_file code is
left thinking it owns the now-closed file descriptor, and will do an
extra close() on the descriptor. But if we do it before, we have the
opposite problem: the lock_file code will close the descriptor, and
fclose() will do the extra close().

We can handle this correctly by using fdopen_lock_file(). That leaves
ownership of the stdio handle with the lock_file, which knows not to
double-close it.

We do have to adjust the code a bit:

  - we have to handle errors ourselves; we can just die(), since that's
    what xfdopen() would have done (and we can even provide a more
    specific error message).

  - we no longer need to call fflush(); committing the lock-file
    auto-closes it, which will now do the flush for us. As a bonus, this
    will actually check that the flush was successful before renaming
    the file into place.

  - we can get rid of the local "fd" variable, since we never look at it
    ourselves now

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-06 08:02:26 -07:00
19ace71de0 sparse-checkout: check commit_lock_file when writing patterns
When writing a new "sparse-checkout" file, we do the usual strategy of
writing to a lockfile and committing it into place. But we don't check
the outcome of commit_lock_file(). Failing there would prevent us from
writing a bogus file (good), but we would ignore the error and return a
successful exit code (bad).

Fix this by calling die(). Note that we need to keep the sparse_filename
variable valid for longer, since the filename stored in the lock_file
struct will be dropped when we run commit_lock_file().

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-06 08:02:26 -07:00
d39cc7185e sparse-checkout: consolidate cleanup when writing patterns
In write_patterns_and_update(), we always need to free the pattern list
before exiting the function.  Rather than handling it manually when we
return early, we can jump to an "out" label where cleanup happens. This
let us drop one line, but also establishes a pattern we can use for
other cleanup.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-06 08:02:26 -07:00
1a60f2066a drop trailing newline from warning/error/die messages
Our error reporting routines append a trailing newline, and the strings
we pass to them should not include them (otherwise we get an extra blank
line after the message).

These cases were all found by looking at the results of:

  git grep -P '[^_](error|error_errno|warning|die|die_errno)\(.*\\n"[,)]' '*.c'

Note that we _do_ sometimes include a newline in the middle of such
messages, to create multiline output (hence our grep matching "," or ")"
after we see the newline, so we know we're at the end of the string).

It's possible that one or more of these cases could intentionally be
including a blank line at the end, but having looked at them all
manually, I think these are all just mistakes.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-05 09:07:12 -07:00
46f6ca2a68 builtin/repack: fix leaking keep-pack list
The list of packs to keep is populated via a command line option but
never free'd. Plug this memory leak.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-05 08:49:13 -07:00
ed78f048ae merge-ort: fix two leaks when handling directory rename modifications
There are two leaks in `apply_directory_rename_modifications()`:

  - We do not release the `dirs_to_insert` string list.

  - We do not release some `conflict_info` we put into the
    `opt->priv->paths` string map.

The former is trivial to fix. The latter is a bit less straight forward:
the `util` pointer of the string map may sometimes point to data that
has been allocated via `CALLOC()`, while at other times it may point to
data that has been allocated via a `mem_pool`.

It very much seems like an oversight that we didn't also allocate the
conflict info in this code path via the memory pool, though. So let's
fix that, which will also plug the memory leak for us.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-05 08:49:13 -07:00
2a01470891 match-trees: fix leaking prefixes in shift_tree()
In `shift_tree()` we allocate two empty strings that we end up
passing to `match_trees()`. If that function finds a better match it
will update these pointers to point to a newly allocated strings,
freeing the old strings. We never free the final results though, neither
the ones we have allocated ourselves, nor the one that `match_trees()`
might've returned to us.

Fix the resulting memory leaks by creating a common exit path where we
free them.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-05 08:49:12 -07:00
68bd0a94be builtin/fmt-merge-msg: fix leaking buffers
Fix leaking input and output buffers in git-fmt-merge-msg(1).

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-05 08:49:12 -07:00
ee087c29c8 builtin/grep: fix leaking object context
Even when `get_oid_with_context()` fails it may have allocated some data
in the object context. But we do not release it in git-grep(1) when the
call fails, leading to a memory leak. Plug it.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-05 08:49:12 -07:00
149c83e0aa builtin/pack-objects: plug leaking list of keep-packs
The `--keep-pack` option of git-pack-objects(1) populates the arguments
into a string list. And while the list is marked as `NODUP` and thus
won't duplicate the strings, the list entries themselves still need to
be free'd. We don't though, causing a leak.

Plug it.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-05 08:49:12 -07:00
860b678016 builtin/repack: fix leaking line buffer when packing promisors
In `repack_promisor_objects()` we read output from git-pack-objects(1)
line by line, using `strbuf_getline_lf()`. We never free the line
buffer, causing a memory leak. Plug it.

This leak is being hit in t5616, but plugging it alone is not
sufficient to make the whole test suite leak free.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-05 08:49:12 -07:00
a46f231975 negotiator/skipping: fix leaking commit entries
When releasing the skipping negotiator we free its priority queue, but
not the contained entries. Fix this to plug a memory leak.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-05 08:49:12 -07:00
16c6fb5a94 shallow: fix leaking members of struct shallow_info
We do not free several struct members in `clear_shallow_info()`. Fix
this to plug the resulting leaks.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-05 08:49:12 -07:00
14c0ea0f6f shallow: free grafts when unregistering them
When removing a graft via `unregister_shallow()` we remove it from the
grafts array, but do not free the structure. Fix this to plug the leak.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-05 08:49:12 -07:00
0d1d22f5a3 object: clear grafts when clearing parsed object pool
We do not clear grafts part of the parsed object pool when clearing the
pool itself, which can lead to memory leaks when a repository is being
cleared.

Fix this by moving `reset_commit_grafts()` into "object.c" and making it
part of the `struct parsed_object_pool` interface such that we can call
it from `parsed_object_pool_clear()`. Adapt `parsed_object_pool_new()`
to take and store a reference to its owning repository, which is needed
by `unparse_commit()`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-05 08:49:11 -07:00
b8849e236f gpg-interface: fix misdesigned signing key interfaces
The interfaces to retrieve signing keys and their IDs are misdesigned as
they return string constants even though they indeed allocate memory,
which leads to memory leaks. Refactor the code to instead always return
allocated strings and let the callers free them accordingly.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-05 08:49:11 -07:00
49d47eb541 send-pack: fix leaking push cert nonce
When retrieving the push cert nonce from the server, we first store the
constant returned by `server_feature_value()` and then, if the nonce is
valid, we duplicate the nonce memory to a NUL-terminated string, so that
we can pass it to `generate_push_cert()`. We never free the latter and
thus cause a memory leak.

Fix this by storing the limited-lifetime nonce into a scope-local
variable such that the long-lived, allocated nonce can be easily freed
without having to cast away its constness.

This leak was exposed by t5534, but fixing it is not sufficient to make
the whole test suite leak free.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-05 08:49:11 -07:00
42c153e1c0 remote: fix leak in reachability check of a remote-tracking ref
In `check_if_includes_upstream()` we retrieve the local ref
corresponding to a remote-tracking ref we want to check reachability
for. We never free that local ref and thus cause a memory leak. Fix
this.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-05 08:49:11 -07:00
cdbb7208c8 remote: fix leaking tracking refs
When computing the remote tracking ref we cause two memory leaks:

  - We leak when `remote_tracking()` fails.

  - We leak when the call to `remote_tracking()` succeeds and sets
    `ref->tracking_ref()`.

Fix both of these leaks.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-05 08:49:11 -07:00
1e8cb17ac5 builtin/submodule--helper: fix leaking refs on push-check
In the push-check subcommand of the submodule helper we acquire a list
of local refs, but never free that list. Fix this memory leak.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-05 08:49:11 -07:00
3eefd348e5 submodule: fix leaking fetch task data
The `submodule_parallel_fetch` structure contains various data
structures that we use to set up parallel fetches of submodules. We do
not free some of its data though, causing memory leaks. Plug those.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-05 08:49:11 -07:00
ac2e7d545e upload-pack: fix leaking child process data on reachability checks
We spawn a git-rev-list(1) command to perform reachability checks in
"upload-pack.c". We do not release memory associated with the process
in error cases though, thus leaking memory.

Fix these by calling `child_process_clear()`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-05 08:49:11 -07:00
7eb6f02c55 builtin/push: fix leaking refspec query result
When appending a refspec via `refspec_append_mapped()` we leak the
result of `query_refspecs()`. The overall logic around refspec queries
is quite weird, as callers are expected to either set the `src` or `dst`
pointers, and then the (allocated) result will be in the respective
other struct member.

As we have the `src` member set, plugging the memory leak is thus as
easy as just freeing the `dst` member. While at it, use designated
initializers to initialize the structure.

This leak was exposed by t5516, but fixing it is not sufficient to make
the whole test suite leak free.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-05 08:49:10 -07:00
e03004f7f8 send-pack: fix leaking common object IDs
We're leaking the array of common object IDs in `send_pack()`. Fix this
by creating a common exit path where we free the leaking data. While at
it, unify some other cleanups now that we have a central place to put
them.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-05 08:49:10 -07:00
63494913ec fetch-pack: fix memory leaks on fetch negotiation
We leak both the `nt_object_array` and `negotiator` structures in
`negotiate_using_fetch()`. Plug both of these leaks.

These leaks were exposed by t5516, but fixing them is not sufficient to
make the whole test suite leak free.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-05 08:49:10 -07:00