When passing `--incremental` to git-blame(1) we exit early by jumping to
the `cleanup` label. But some of the cleanups we perform are handled
between the `goto` and its label, and thus we leak the data.
Move the cleanups after the `cleanup` label. While at it, move the logic
to free the scoreboard's `final_buf` into `cleanup_scoreboard()` and
drop its `const` declaration.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This one is a little bit more curious. In t6112, we have a test that
exercises the `git rev-list --filter` option with invalid filters. We
execute git-rev-list(1) via `test_must_fail`, which means that we check
for leaks even though Git exits with an error code. This causes the
following leak:
Direct leak of 27 byte(s) in 1 object(s) allocated from:
#0 0x5555555e6946 in realloc.part.0 lsan_interceptors.cpp.o
#1 0x5555558fb4b6 in xrealloc wrapper.c:137:8
#2 0x5555558b6e06 in strbuf_grow strbuf.c:112:2
#3 0x5555558b7550 in strbuf_add strbuf.c:311:2
#4 0x5555557c1a88 in strbuf_addstr strbuf.h:310:2
#5 0x5555557c1d4c in parse_list_objects_filter list-objects-filter-options.c:261:3
#6 0x555555885ead in handle_revision_pseudo_opt revision.c:2899:3
#7 0x555555884e20 in setup_revisions revision.c:3014:11
#8 0x5555556c4b42 in cmd_rev_list builtin/rev-list.c:588:9
#9 0x5555555ec5e3 in run_builtin git.c:483:11
#10 0x5555555eb1e4 in handle_builtin git.c:749:13
#11 0x5555555ec001 in run_argv git.c:819:4
#12 0x5555555eaf94 in cmd_main git.c:954:19
#13 0x5555556fd569 in main common-main.c:64:11
#14 0x7ffff7ca714d in __libc_start_call_main (.../lib/libc.so.6+0x2a14d)
#15 0x7ffff7ca7208 in __libc_start_main@GLIBC_2.2.5 (.../libc.so.6+0x2a208)
#16 0x5555555ad064 in _start (git+0x59064)
This leak is valid, as we call `die()` and do not clean up the memory at
all. But what's curious is that this is the only leak reported, because
we don't clean up any other allocated memory, either, and I have no idea
why the leak sanitizer treats this buffer specially.
In any case, we can work around the leak by shuffling things around a
bit. Instead of calling `gently_parse_list_objects_filter()` and dying
after we have modified the filter spec, we simply do so beforehand. Like
this we don't allocate the buffer in the error case, which makes the
reported leak go away.
It's not pretty, but it manages to make t6112 leak free.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The `obuf` member of `struct merge_options` is used to buffer output in
some cases. In order to not discard its allocated memory we only release
its contents in `merge_finalize()` when we're not currently recursing
into a subtree.
This results in some situations where we seemingly do not release the
buffer reliably. We thus have calls to `strbuf_release()` for this
buffer scattered across the codebase. But we're missing one callsite in
git-merge(1), which causes a memory leak.
We should ideally refactor this interface so that callers don't have to
know about any such internals. But for now, paper over the issue by
adding one more `strbuf_release()` call.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We use `repo_config_get_string()` to read "status.showUntrackedFiles"
from the config subsystem. This function allocates the result, but we
never free the result after parsing it.
The value never leaves the scope of the calling function, so refactor it
to instead use `repo_config_get_string_tmp()`, which does not hand over
ownership to the caller.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We never release the local `struct strbuf base` buffer, thus leaking
memory. Fix this leak.
This leak is exposed by t7063, but plugging it alone does not make the
whole test suite pass.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
While "common-main.c" already initializes `the_repository` for us, we do
so a second time in the "read-cache" test helper. This causes a memory
leak because the old repository's contents isn't released.
Stop calling `initialize_repository()` to plug this leak.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
While we free the `fsmonitor_dirty` member of `struct index_state`, we
do not free the contents of that EWAH. Do so by using `ewah_free()`
instead of `FREE_AND_NULL()`.
This leak is exposed by t7519, but plugging it alone does not make the
test suite pass.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There are several cases where we invalidate untracked cache directory
entries where we do not free the underlying data, but reset the number
of entries. This causes us to leak memory because `free_untracked()`
will not iterate over any potential entries which we still had in the
array.
Fix this issue by freeing old entries. The leak is exposed by t7519, but
plugging it alone does not make the whole test suite pass.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The `cnt` variable tracks the number of lines in a patch diff. It can
happen though that there are no newlines, in which case we'd still end
up allocating our array of `sline`s. In fact, we always allocate it with
`cnt + 2` entries: one extra entry for the deletion hunk at the end, and
another entry that we don't seem to ever populate at all but acts as a
kind of sentinel value.
When we loop through the array to clear it at the end of this function
we only loop until `lno < cnt`, and thus we may not end up releasing
whatever the two extra `sline`s contain. While that shouldn't matter for
the sentinel value, it does matter for the extra deletion hunk sline.
Regardless of that, plug this memory leak by releasing both extra
entries, which makes the logic a bit easier to reason about.
While at it, fix the formatting of a local comment, which incidentally
also provides the necessary context for why we overallocate the `sline`
array.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We do not free the key ID when signing a tag fails. Do so by using
the common exit path.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix leaking import and export marks for transport helpers.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The cleanup string set by the config is leaking when it is being
overridden by an option. Fix this by tracking these via two separate
variables such that we can free the old value.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When formatting trailer lines we iterate through each of the trailers
and munge their respective token/value pairs according to the trailer
options. When formatting a trailer that has its `item->token` pointer
set we perform the munging in two local buffers. In the case where we
figure out that the value is empty and `trim_empty` is set we just skip
over the trailer item. But the buffers are local to the loop and we
don't release their contents, leading to a memory leak.
Plug this leak by lifting the buffers outside of the loop and releasing
them on function return. This fixes the memory leaks, but also optimizes
the loop as we don't have to reallocate the buffers on every single
iteration.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix leaking trailer values when replacing the value with a command or
when the token value is empty.
This leak is exposed by t7513, but plugging it does not make the whole
test suite pass.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
While we free the worktree change data, we never free its contents. Fix
this.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We don't clear `struct upload_pack::uri_protocols`, which causes a
memory leak. Fix this.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The signature check in the formatting context is never getting released.
Fix this to plug the resulting memory leak.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In `do_diff_cache()` we initialize a new `rev_info` and then overwrite
its `diffopt` with a user-provided set of options. This can leak memory
because `repo_init_revisions()` may end up allocating memory for the
`diffopt` itself depending on the configuration. And since that field is
overwritten we won't ever free it.
Plug the memory leak by releasing the diffopts before we overwrite them.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The memory allocated by `prepare_to_use_bloom_filter()` is not released
by `release_revisions()`, causing a memory leak. Plug it.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When executing with `--max-count=0` we'll return early from git-grep(1)
without performing any cleanup, which causes memory leaks. Plug these.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In `grep_splice_or()` we search for the next `TRUE` node in our tree of
grep expressions and replace it with the given new expression. But we
don't free the old node, which causes a memory leak. Plug it.
This leak is exposed by t7810, but plugging it alone isn't sufficient to
make the test suite pass.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "reach" test tool doesn't bother to clean up any of its allocated
resources, causing various leaks. Plug them.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The list of server options populated via `OPT_STRING_LIST()` is never
cleared, causing a memory leak. Plug it.
This leak is exposed by t5702, but plugging it alone does not make the
whole test suite pass.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The dumb-http code regressed when the result of re-indexing a pack
yielded an *.idx file that differs in content from the *.idx file it
downloaded from the remote. This has been corrected by no longer
relying on the *.idx file we got from the remote.
* jk/dumb-http-finalize:
packfile: use oidread() instead of hashcpy() to fill object_id
packfile: use object_id in find_pack_entry_one()
packfile: convert find_sha1_pack() to use object_id
http-walker: use object_id instead of bare hash
packfile: warn people away from parse_packed_git()
packfile: drop sha1_pack_index_name()
packfile: drop sha1_pack_name()
packfile: drop has_pack_index()
dumb-http: store downloaded pack idx as tempfile
t5550: count fetches in "previously-fetched .idx" test
midx: avoid duplicate packed_git entries
Replace various calls to atoi() with strtol_i() and strtoul_ui(), and
add improved error handling.
* ua/atoi:
imap: replace atoi() with strtol_i() for UIDVALIDITY and UIDNEXT parsing
merge: replace atoi() with strtol_i() for marker size validation
daemon: replace atoi() with strtoul_ui() and strtol_i()
Teach 'git notes add' and 'git notes append' a new '-e' flag,
instructing them to open the note in $GIT_EDITOR before saving.
* sa/notes-edit:
notes: teach the -e option to edit messages in editor
Documentation update to clarify that 'uploadpack.allowAnySHA1InWant'
implies both 'allowTipSHA1InWant' and 'allowReachableSHA1InWant'.
* ps/upload-pack-doc:
doc: document how uploadpack.allowAnySHA1InWant impact other allow options
Treat ECONNABORTED the same as ECONNRESET in 'git credential-cache' to
work around a possible Cygwin regression. This resolves a race condition
caused by changes in Cygwin's handling of socket closures, allowing the
client to exit cleanly when encountering ECONNABORTED.
* rj/cygwin-exit:
credential-cache: treat ECONNABORTED like ECONNRESET
Test update.
* ua/t3404-cleanup:
t3404: replace test with test_line_count()
t3404: avoid losing exit status with focus on `git show` and `git cat-file`
Various platform compatibility fixes split out of the larger effort
to use Meson as the primary build tool.
* ps/platform-compat-fixes:
t6006: fix prereq handling with `test_format ()`
http: fix build error on FreeBSD
builtin/credential-cache: fix missing parameter for stub function
t7300: work around platform-specific behaviour with long paths on MinGW
t5500, t5601: skip tests which exercise paths with '[::1]' on Cygwin
t3404: work around platform-specific behaviour on macOS 10.15
t1401: make invocation of tar(1) work with Win32-provided one
t/lib-gpg: fix setup of GNUPGHOME in MinGW
t/lib-gitweb: test against the build version of gitweb
t/test-lib: wire up NO_ICONV prerequisite
t/test-lib: fix quoting of TEST_RESULTS_SAN_FILE
Avoid losing exit status by having Git command being tested on the
upstream side of a pipe.
* co/t6050-pipefix:
t6050: avoid pipes with upstream Git commands
Teaches the ref-filter machinery to recognize and avoid cases where
sorting would be redundant.
* ps/ref-filter-sort:
ref-filter: format iteratively with lexicographic refname sorting
Implements a new reftable-specific strbuf replacement to reduce
reftable's dependency on Git-specific data structures.
* ps/reftable-strbuf:
reftable: handle trivial `reftable_buf` errors
reftable/stack: adapt `stack_filename()` to handle allocation failures
reftable/record: adapt `reftable_record_key()` to handle allocation failures
reftable/stack: adapt `format_name()` to handle allocation failures
t/unit-tests: check for `reftable_buf` allocation errors
reftable/blocksource: adapt interface name
reftable: convert from `strbuf` to `reftable_buf`
reftable/basics: provide new `reftable_buf` interface
reftable: stop using `strbuf_addf()`
reftable: stop using `strbuf_addbuf()`
In df383b5842 (t/test-lib: wire up NO_ICONV prerequisite, 2024-10-16) we
have introduced a new NO_ICONV prerequisite that makes us skip tests in
case Git is not compiled with support for iconv. This change subtly
broke t6006: while the test suite still passes, some of its tests won't
execute because they run into an error.
./t6006-rev-list-format.sh: line 92: test_expect_%e: command not found
The broken tests use `test_format ()`, and the mentioned commit simply
prepended the new prerequisite to its arguments. But that does not work,
as the function is not aware of prereqs at all and will now treat all of
its arguments incorrectly.
Fix this by making the function aware of prereqs by accepting an
optional fourth argument. Adapt the callsites accordingly.
Reported-by: Josh Steadmon <steadmon@google.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
When chasing a REF_DELTA, we need to pull the raw hash bytes out of the
mmap'd packfile into an object_id struct. We do that with a raw
hashcpy() of the appropriate length (that happens directly now, though
before the previous commit it happened inside find_pack_entry_one(),
also using a hashcpy).
But I think this creates a potentially dangerous situation due to
d4d364b2c7 (hash: convert `oidcmp()` and `oideq()` to compare whole
hash, 2024-06-14). When using sha1, we'll have uninitialized bytes in
the latter part of the object_id.hash buffer, which could fool oideq(),
etc.
We should use oidread() instead, which correctly zero-pads the extra
bytes, as of c98d762ed9 (global: ensure that object IDs are always
padded, 2024-06-14).
As far as I can see, this has not been a problem in practice because the
object_id we feed to find_pack_entry_one() is never used with oideq(),
etc. It is being compared to the bytes mmap'd from a pack idx file,
which of course do not have the extra padding bytes themselves. So
there's no bug here, but this just puzzled me while looking at the code.
We should do the more obviously safe thing, both for future-proofing and
to avoid confusing readers.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
The main function we use to search a pack index for an object is
find_pack_entry_one(). That function still takes a bare pointer to the
hash, despite the fact that its underlying bsearch_pack() function needs
an object_id struct. And so we end up making an extra copy of the hash
into the struct just to do a lookup.
As it turns out, all callers but one already have such an object_id. So
we can just take a pointer to that struct and use it directly. This
avoids the extra copy and provides a more type-safe interface.
The one exception is get_delta_base() in packfile.c, when we are chasing
a REF_DELTA from inside the pack (and thus we have a pointer directly to
the mmap'd pack memory, not a struct). We can just bump the hashcpy()
from inside find_pack_entry_one() to this one caller that needs it.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
