Various tests in 't7900-maintenance.sh' have been updated to use a
throwaway repository, and auto-detaching of maintenance tasks is now
disabled for these tests to fix flaky races with concurrent background
maintenance jobs.
* ps/t7900-deflake-maintenance:
t7900: fix flaky "maintenance.strategy" test
t7900: adapt some tests to use a throwaway repository
A client requesting the promisor-remote capability without a value
caused a null pointer dereference, which has been corrected by
rejecting a request without an argument.
* en/serve-promisor-remote-fix:
serve: reject valueless promisor-remote capability
The 'pack-objects' and delta-encoding code paths have been updated to
use 'size_t' instead of 'unsigned long' for object sizes and offset
limits, avoiding potential truncation issues on 64-bit Windows.
* js/pack-objects-delta-size-t:
packfile: widen `unpack_object_header_buffer()` to `size_t`
git-zlib: widen `git_deflate_bound()` to `size_t`
t/helper/test-pack-deltas: widen `do_compress()`'s maxsize local to `size_t`
http-push: widen `start_put()`'s size local from `ssize_t` to `size_t`
diff: widen `deflate_it()`'s bound local from int to `size_t`
archive-zip: widen `zlib_deflate_raw()`'s maxsize local to `size_t`
packfile, git-zlib: widen `use_pack()` and zstream avail fields to `size_t`
delta: widen `create_delta()` and `diff_delta()` to `size_t`
pack-objects: widen `mem_usage` and `try_delta()`'s out-param to `size_t`
pack-objects: widen `free_unpacked()` return to `size_t`
pack-objects: widen delta-cache accounting to `size_t`
delta: widen `create_delta_index()` parameter to `size_t`
diff-delta: widen `struct delta_index`' size fields to `size_t`
The '--shallow-file' option of 'git' command requires a value, but the
code did not check the presence of a value and instead segfaulted
without one, which has been corrected.
* cc/git-shallow-file-wo-value:
git: avoid segfault on "git --shallow-file" without a value
The setting of a now-unused member '.pretty_given' in the sequencer
machinery has been removed.
* en/sequencer-lose-pretty-given:
sequencer: remove unnecessary variable setting
A handful of code paths have been corrected to check return values
from functions like curl_easy_duphandle(), deflateInit(), lseek(),
dup(), and strbuf_getline_lf(), resolving several Coverity warnings
about unchecked returns.
* js/coverity-unchecked-returns-fix:
bisect: handle dup() failure when redirecting stdout
bisect: check get_terms return at all call sites
bisect: check strbuf_getline_lf return when reading terms
transport-helper: warn when export-marks file cannot be finalized
transport-helper: check dup() return in get_exporter
compat/pread: check initial lseek for errors
last-modified: handle repo_parse_commit() failures
reftable tests: check reftable_table_init_ref_iterator() return
reftable/block: check deflateInit() return value
reftable: handle block-writer initialization errors
config: propagate launch_editor() failure in show_editor()
http: die on curl_easy_duphandle failure in get_active_slot
The merge-base computation has been optimized by stopping the walk
early when one side's exclusive commits in the queue are exhausted,
yielding significant speedups for queries with one-sided histories.
* kk/merge-base-exhaustion:
commit-reach: remove commit-date ordering fallback
commit-reach: move min_generation check into paint_queue_get()
commit-reach: terminate merge-base walk when one paint side is exhausted
commit-reach: introduce struct paint_state with per-side counters
t6600: add clock-skew topologies and step counts for edge cases
commit-reach: add trace2 instrumentation to paint_down_to_common()
t6099: add side-exhaustion regression test
t6600: add test cases for side-exhaustion edge cases
test-lib-functions: improve diagnostic output for trace2 data assertions
Documentation/technical: add paint-down-to-common doc
The sequencer has been updated to release the object database before
spawning 'git commit'. This prevents open file handles from
blocking auto-maintenance tasks, such as repacking, on systems like
Windows where open files cannot be easily unlinked.
* js/sequencer-release-odb-before-commit:
sequencer: release the ODB before spawning git commit
The error message given by 'git send-email' when a message file is
missing a 'Subject:' header has been clarified, and the error string
is now terminated with a newline so that Perl avoids appending its
internal source location data.
* hn/send-email-missing-subject-error:
send-email: clarify missing subject error
The 'struct odb_read_stream' and 'struct odb_write_stream'
structures have been consolidated into a single unified 'struct
odb_stream' structure, simplifying object database streaming APIs
and enabling streaming of arbitrary object types.
* ps/odb-streams:
odb/streaming: unify function names to create new streams
odb/streaming: rename `struct input_zstream_data`
odb/streaming: rename `struct read_object_fd_data`
odb/streaming: consolidate read and write streams
odb/streaming: rename `struct odb_read_stream`
odb/streaming: support streaming arbitrary object types
odb/streaming: drop `is_finished` field
odb/streaming: track write stream size in the structure
The usage string of 'git fast-import' has been updated to use the
parse_options() API for displaying help, and its SYNOPSIS in the
documentation has been standardized to match.
* cc/fast-import-usage:
fast-import: remove useless from_stream argument
fast-import: use parse_options() for command line options
fast-import: use callbacks to parse some options
fast-import: use struct option for usage string
fast-import: move command state globals into 'struct fast_import_state'
fast-import: introduce 'struct fast_import_state'
fast-import: factor out option_*() functions
fast-import: use int for some bool flags
fast-import: localize 'i' into the 'for' loops using it
api-parse-options.adoc: document hidden and OPT_*_F option macros
api-parse-options.adoc: document per-option flags
parse-options: introduce OPT_HIDDEN_GROUP
The 'remote-object-info' command for 'git cat-file --batch-command'
has been extended to support the '%(objecttype)' placeholder.
* ps/cat-file-remote-object-info-type:
cat-file: unify default format
serve: advertise type capability
fetch-object-info: parse type from server response
protocol-caps: add type support to object-info
transport: drop remote object-info fields from transport struct
fetch-object-info: die() on the remaining error path
fetch-object-info: use dedicated struct for the results
fetch-object-info: pass arguments directly instead of a struct
fetch-object-info: detect malformed server responses
t5701: use test_file_size() to get the size of a file
'git add' has been taught a new '--resolved' option to stage
conflict-resolved paths, while leaving unrelated local changes
unstaged. It scans the unmerged paths for leftover conflict
markers and aborts if any are found.
* jc/add-resolved:
add: introduce '--resolved' option
read-cache: add remove_file_from_index_with_flags()
merge-ll: consolidate conflict marker scanning logic
read-cache: reindent
The 'ssh-agent' tests in 't7528' have been fixed to work when the
user's login shell is csh-like, by explicitly passing '-s' to
'ssh-agent' to force Bourne shell syntax.
* kl/t7528-ssh-agent-for-csh-users:
t7528: fix failure under csh
Concurrent downloads of packfiles via packfile URIs and dumb HTTP are
safer by avoiding concurrent appends to the staging file. Opening in
read-write mode with separate file offsets prevents corruption and
preserves resumability. 'fetch-pack' now tolerates pre-existing
'.keep' files.
* tn/packfile-uri-concurrency:
fetch-pack: accept "pack" output for packfile URIs
http: permit unlinking partial packs on Windows
http: avoid concurrent appends to partial packs
http: accept HTTP 416 for complete partial packs
http: avoid closing index-pack input twice
http-fetch: correct --index-pack-arg documentation
The 'TRACE2_ANCESTRY' prerequisite in the 't0213' test script has been
refined to avoid failures under user-mode emulation by verifying that
the ancestry collector reports the expected process names rather than
the emulator binary name.
* jm/t0213-skip-emulated-ancestry-tests:
t0213: skip ancestry tests under user-mode emulation
Documentation for 'git interpret-trailers' has been updated to explain
the format of trailer keys (alphanumeric characters and hyphens),
replace outdated terminology, define key terms upfront, and document
how comment lines in the input are treated.
* kh/doc-trailers:
doc: interpret-trailers: document comment line treatment
doc: interpret-trailers: rewrite new-trailers paragraphs
doc: interpret-trailers: commit to “trailer block” term
doc: interpret-trailers: join new-trailers again
doc: interpret-trailers: add key format example
doc: interpret-trailers: explain key format
doc: interpret-trailers: explain the format after the intro
doc: interpret-trailers: not just for commit messages
doc: interpret-trailers: use “metadata” in Name as well
doc: interpret-trailers: replace “lines” with “metadata”
doc: interpret-trailers: stop fixating on RFC 822
The creation of the on-disk data structures for the object database
has been made pluggable, allowing future backends to customize their
setup. As part of this, the initialization of the object database
has been deferred, and the loading of the loose-object map has been
detangled from repository initialization.
* ps/odb-make-creation-pluggable:
odb: make creation of on-disk structures pluggable
odb/source: introduce function to map source type to name
setup: defer object database creation
setup: handle ODB-related environment variables in `odb_new()`
setup: detangle loading of loose object maps
loose: load loose object map for the correct source
The 'git branch' command has been taught the '--delete-merged' option
to remove local branches that are already merged into their tracked
remote-tracking branches.
* hn/branch-delete-merged:
branch: add --dry-run for --delete-merged
branch: add branch.<name>.deleteMerged opt-out
branch: add --delete-merged <pattern>
branch: prepare delete_branches for a bulk caller
branch: let delete_branches skip unmerged branches on bulk refusal
branch: convert delete_branches() to a flags argument
branch: add --forked filter for --list mode
As part of the ongoing effort to replace `unsigned long` data types with
`size_t` wherever appropriate (mainly to fix all those problems on
Windows with objects larger than 4GB), let's also adjust the return type
and the type of the `len` parameter of this function.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
All four `unsigned long`/`int`/`ssize_t` receivers across archive-zip,
diff, http-push and t/helper/test-pack-deltas were widened to `size_t`
in the prior commits, and remote-curl and fast-import were already
there. With every caller prepared, both the parameter and the return
type can now move without introducing any silent narrowing.
For inputs above zlib's `uLong` range (i.e. >4 GiB on platforms where
`uLong` is 32-bit, notably 64-bit Windows), defer to zlib's stored-block
formula (the same fallback it would itself use, see
https://github.com/madler/zlib/blob/v1.3.2/deflate.c#L832-L928 keeping
in mind that for large sizes, the `storelen` would be relevant, also
compare with https://github.com/madler/zlib/issues/549 for a fuller
story) plus the worst-case wrapper overhead. The existing path through
`deflateBound()` is unchanged for inputs that fit.
Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Prep for the upcoming `git_deflate_bound()` widening to `size_t`. The
local is only ever the return value of `git_deflate_bound()` and the
`xmalloc()`/`stream.avail_out` sizes derived from it; widening it has no
semantic effect today.
Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The local is initialised from `git_deflate_bound()` (an unsigned upper
bound on the deflated output, never negative) and used in exactly three
places: the initialising assignment, `strbuf_grow(buf, size)` whose
parameter is already `size_t`, and `stream.avail_out` which became
`size_t` in the prior commit. There is no comparison against zero or a
negative value, no subtraction, no arithmetic that depends on
signedness, and no path that would assign a signed quantity to it.
The original `ssize_t` was the wrong type to begin with: a
`git_deflate_bound()` result above `SSIZE_MAX` would have wrapped
negative on assignment and then implicitly re-extended to a huge
`size_t` at `strbuf_grow()`/`stream.avail_out`, requesting an absurd
allocation. That is not a real-world concern for the object sizes
http-push pushes today, but it is also the reason the type needs to move
to `size_t` before `git_deflate_bound()` itself is widened.
Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fixes a pre-existing silent narrowing from `git_deflate_bound()`'s
`unsigned long` return into an `int` local: anything past 2 GiB has
always wrapped negative here and then been re-extended to `size_t`
inside `xmalloc()`. Also prep for the upcoming `git_deflate_bound()`
widening to `size_t`, which would extend the narrowing further if
`bound` stayed `int`.
Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Prep for the upcoming `git_deflate_bound()` widening to `size_t`: the
local that catches its return needs to be `size_t` too, otherwise the
widening would introduce a silent Windows narrowing here. No semantic
effect with the current unsigned-long-returning `git_deflate_bound()`
(`size_t == unsigned long` on this caller's platforms today).
Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Bundling the two widenings: four call sites pass `&stream.avail_in`
directly to `use_pack()`, and widening either type fencepost alone would
force a bridge variable at each. Doing both together is the simpler end
state and is the prerequisite for the `do_compress()` widening in the
next commit, which is what lets `write_no_reuse_object()` lose its last
`cast_size_t_to_ulong()` shim.
The unsigned-long locals widened at the other `use_pack()` callers
(avail / remaining / left) hold pack-window sizes bounded by
`core.packedGitWindowSize`, so the change is type consistency rather
than a new >4GB capability. `git_zstream.avail_in`/`avail_out` likewise
reach zlib's `uInt` fields only after `zlib_buf_cap()`'s 1 GiB cap, so
the wrapper already accepted `size_t`-shaped inputs in practice.
Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Last stop in the delta-encoding API widening for >4 GiB blobs on
Windows: with `create_delta_index()` done in the prior commit and
`create_delta()`/`diff_delta()` finished here, every byte count that
crosses delta.h is now `size_t`. The struct fields they store into have
been `size_t` since the diff-delta struct widening.
The API change must move with all callers in the same commit (the build
only passes when every `&delta_size` matches the new `size_t*`). Caller
updates are kept minimal:
* builtin/pack-objects.c `get_delta()` and `try_delta()`: widen only
the local `delta_size` variable; the surrounding unsigned-long
locals and their `cast_size_t_to_ulong()` shims are out of scope
here and will be cleaned up in their own commits.
* builtin/fast-import.c, diff.c, t/helper/test-pack-deltas.c:
keep the local unsigned-long delta size (each feeds a still-
unsigned-long downstream consumer: zlib's `avail_in`,
`deflate_it()`, the test helper's own `do_compress()`), and bridge
via a temporary `size_t` plus `cast_size_t_to_ulong()`. The new
casts are paid back in later topics that widen those consumers.
* t/helper/test-delta.c: widen the local outright (no downstream
consumer beyond the test's own `out_size`, which is already
`size_t`).
Note that GCC struggles a bit to figure out that `deltalen` is always
initialized before it is used; To help it along, we initialize it to 0.
This work-around will go away in a later patch series when `deltalen`
can be widened to `size_t`.
Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The pair must move together because `find_deltas()` passes `&mem_usage`
to `try_delta()`: widening either alone breaks the type match.
`mem_usage` accumulates per-object byte counts already computed in
`size_t` (`SIZE()` and `sizeof_delta_index()` reach here through
`free_unpacked()`, now `size_t`), and was the last 32-bit-on-Windows
narrowing point in the delta-window memory accounting chain. With this
commit, that chain uses `size_t` consistently except for
`sizeof_delta_index()`'s still-narrow return, whose value is bounded by
`create_delta_index()`'s entries cap.
`window_memory_limit` (config-driven via `git_config_ulong()`) stays
`unsigned long`: it is only compared against `mem_usage` and promotes.
Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
`free_unpacked()` sums two byte counts: `sizeof_delta_index()` and
`SIZE(n->entry)`. The latter has been `size_t` since the prior topic
"More work supporting objects larger than 4GB on Windows" widened
`SIZE()`/`oe_size()` to `size_t`, so accumulating it into an `unsigned
long` return was a silent Windows-only truncation on a packing run with
many large objects.
The sole caller, `find_deltas()`, still holds its own `mem_usage` in an
`unsigned long` for now, and therefore still truncates silently.
Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
These three are a single accounting tuple (the globals tracking
cumulative cached-delta bytes, plus the helper that compares them
against an incoming delta size) and are latently 32-bit on Windows where
`unsigned long` != `size_t`: a pack with many large cached deltas could
wrap silently.
The widening is internally consistent on its own: the additions and
subtractions against delta_cache_size already come from `size_t` sources
(`DELTA_SIZE()` returns `size_t`), and `delta_cacheable()`'s sole caller
in `try_delta()` still passes `unsigned long`, which promotes.
Prerequisite for dropping `try_delta()`'s `cast_size_t_to_ulong()`
shims, which becomes possible once 1create_delta()` and `diff_delta()`
are widened in a later commit.
Note: since `max_delta_cache_size` changes data type to `size_t`, a pair
of new helpers is introduced to parse config values of that type, too.
Assisted-by: Opus 4.7
Helped-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The sole caller (`try_delta()` in builtin/pack-objects.c) passes an
`unsigned long`, which promotes safely, so no caller fixups are needed.
Splitting it out keeps the `diff_delta()`/`create_delta()` widening,
which does ripple to several callers, in its own commit.
Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Preparation for widening the delta-encoding API to `size_t` in
subsequent commits, which is what lets pack-objects drop the
`cast_size_t_to_ulong()` shims that 606c192380 (odb, packfile: use
size_t for streaming object sizes, 2026-05-08) had to leave behind in
`get_delta()` and `try_delta()` because their downstream consumers were
still narrow.
The struct is private to diff-delta.c, so widening its fields in
isolation is a no-op at runtime: the values stored continue to fit in 32
bits on Windows because the public API around it still truncates.
Splitting it out keeps the API-change commit focused on caller updates.
Since the `memsize` attribute is returned by the `sizeof_delta_index()`
function verbatim, that function's return type is adjusted, too.
Assisted-by: Opus 4.7
Helped-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
As of 4557f1add2 (rebase--helper: add a builtin helper for interactive
rebases, 2017-02-09), continuing an interactive rebase uses the builtin
sequencer, which spawns `git commit`.
The child may trigger auto-maintenance, which may need to replace files
for which the sequencer still holds resources. See
https://github.com/git-for-windows/git/issues/6315: on Windows, this
produces unlink retry prompts that cannot succeed while the sequencer
waits for the child.
Resources such as file handles or memory mappings must be released
before spawning a command that may run auto-maintenance, as established
by 28d04e1ec1 (run-command: offer to close the object store before
running, 2021-09-09): release the ODB file handles and memory mappings,
so that auto-gc can repack (potentially deleting existing packfiles in
the process); If the sequencer needs to access the ODB afterwards, it
will gracefully (re-)open the ODB.
Release the sequencer's ODB before spawning `git commit`. The regression
test uses the legacy-delete trick introduced by 69ed0e35a7 (mingw:
optionally use legacy (non-POSIX) delete semantics, 2026-05-07) to
trigger the failure on modern Windows.
Assisted-by: GPT-5.6 Sol
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
To capture the output of each verdict command, bisect_run()
temporarily redirects stdout to a temporary file via the classic
dup(1) / dup2() pair, restoring it afterwards. The return value of
dup(1) is not checked, however. When it fails, the saved descriptor
is -1, which is then passed to close() (the issue Coverity flags),
and the matching dup2() that is meant to restore stdout also fails,
leaving the process with stdout still pointing at the temporary file
for the remainder of the run.
Treat a failed dup(1) or dup2(..., 1) as a fatal error for this bisect
step: close the temporary file descriptor, report the error via
error_errno(), and break out of the loop so the existing cleanup path
handles the rest, just as on other failure paths in this function.
Reported by Coverity as CID 1508242 ("Improper use of negative
value").
Assisted-by: Opus 4.7
Helped-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Six callers of get_terms() silently discard its return value. When
get_terms fails (missing or truncated BISECT_TERMS file), the term
strings remain NULL or empty, causing confusing downstream
behavior: commands like "bisect next" or "bisect run" proceed with
empty term strings, producing nonsensical ref names (refs/bisect/
with no suffix) and misleading error messages.
Let's not discard the return value, but handle an error with the same
message `bisect_terms()` already uses when reading the terms failed.
Pointed out by Coverity.
There is one slight complication here: One caller _needs_ the return
value to indicate an error when the `BISECT_TERMS` file is absent, all
the other call sites are totally okay with a "missing" `BISECT_TERMS`
file. To address that, extend the function signature of `get_terms()` to
indicate which behavior the caller wants.
Assisted-by: Claude Opus 4.6
Helped-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
get_terms() in builtin/bisect.c and read_bisect_terms() in
bisect.c both read the BISECT_TERMS file but do not check the
strbuf_getline_lf() return values. If the file is truncated
(e.g., a partial write from a crash or disk-full condition),
strbuf_getline_lf returns EOF and the strbuf remains empty.
strbuf_detach then returns an empty string, and the term names
silently become "" instead of the expected "bad"/"good" or
custom terms.
In get_terms(), check for EOF and return -1 on truncation,
matching the existing -1 return for a missing file.
In read_bisect_terms(), die with a descriptive message when a
line cannot be read, consistent with the die_errno for a
non-ENOENT open failure in the same function. Unlike get_terms(),
read_bisect_terms() returns void and uses die() for all error
paths, so the die is the appropriate error handling here.
Pointed out by Coverity.
Assisted-by: Claude Opus 4.6
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When push_refs_with_export() finalizes a successful push, it writes
the fast-export marks file to a .tmp sibling and rename()s it into
place. The return value of rename() is currently ignored. If the
rename fails (permission denied, full disk, or an antivirus product
locking the destination on Windows), the .tmp file is left behind
and the existing export_marks file remains stale; the next
fast-export operation that resumes from it then silently operates on
inconsistent bookkeeping.
The push itself succeeded by that point, so promoting this to a
fatal error would be inappropriate. Emit warning_errno() naming both
paths so the user can recover manually, and keep returning 0.
Flagged by Coverity as CID 1427723 ("Unchecked return value").
Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
get_exporter() duplicates helper->in via dup() and stores the
result in fastexport->out. If dup() fails (fd exhaustion), it
returns -1. The child_process machinery interprets out = -1 as
"create a pipe for stdout", which would silently change the
fast-export process's output wiring: instead of sending data
back through the helper's input fd, it would write to a new pipe
that nobody reads from.
Check the return value and report the error before proceeding.
Pointed out by Coverity.
Assisted-by: Claude Opus 4.6
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
git_pread() saves the current file offset via lseek(fd, 0,
SEEK_CUR) and later restores it. If the initial lseek fails
(e.g., the fd is a pipe or otherwise non-seekable),
current_offset is -1. This negative value is later passed to
lseek(fd, -1, SEEK_SET) at line 16, which sets the file position
to an unintended location (or fails with EINVAL on some
platforms).
Check the initial lseek return value and return -1 immediately
if it fails, consistent with the error handling for the other
lseek calls in the same function.
Pointed out by Coverity.
Assisted-by: Claude Opus 4.6
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
last_modified_run() and process_parent() call repo_parse_commit()
without checking the return value at three sites. When a commit
object is corrupt or unavailable (e.g., a shallow clone boundary
or a missing object in a partial clone), the parse fails and the
commit's internal fields (parents, tree, date) are not populated.
The consequences depend on which call site fails:
At line 417 (the main walk loop), c->parents stays NULL after a
failed parse. The parent-walking loop at line 440 simply does not
execute, silently treating the unparsable commit as a root commit.
This produces incorrect "last modified" results: paths changed in
ancestors beyond the corrupt commit are attributed to the wrong
commit or not reported at all.
At line 423 (the --not exclusion walk), n->parents stays NULL,
causing the exclusion walk to stop prematurely. Commits that
should be excluded from the output may be incorrectly included.
At line 293 (process_parent), the parent's tree and parents are
unavailable, so diff operations against it produce wrong results
and the parent's own ancestors are never enqueued for walking.
Skip unparsable commits by checking the return value and
continuing to the next iteration (or returning early in
process_parent). This matches the defensive pattern used in other
revision walkers such as limit_list() and get_revision_internal().
Pointed out by Coverity.
Assisted-by: Claude Opus 4.6
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
test_reftable_table__seek_once() and test_reftable_table__reseek()
both call reftable_table_init_ref_iterator() without checking its
return value. This function returns an int error code (0 on
success, negative on failure). Every other reftable function call
in these same tests checks the return via cl_assert_equal_i() or
cl_assert(), making this omission inconsistent.
If the iterator initialization ever fails (e.g., due to a memory
allocation failure in the reftable internals), the test would
proceed to seek and read with an uninitialized iterator, producing
misleading test results or crashes rather than a clear assertion
failure.
Check the return value via cl_assert_equal_i(ret, 0), consistent
with the surrounding code.
Pointed out by Coverity.
Assisted-by: Claude Opus 4.6
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
block_writer_init() allocates a z_stream and calls deflateInit()
to prepare it for compressing log records. The return value of
deflateInit() is silently discarded. If zlib initialization fails
(e.g., Z_MEM_ERROR when the system is under memory pressure), the
z_stream is left in an undefined state.
Subsequent deflate() calls in block_writer_finish() then operate
on this uninitialized stream. Current zlib/zlib-ng versions handle
such a stream gracefully, by returning `Z_STREAM_ERROR`, so in
practice it would likely not result in catastrophic error.
The function already uses REFTABLE_ZLIB_ERROR for deflate() failures
later in the code path, so returning the same error code for
deflateInit() failure is consistent.
Pointed out by Coverity.
Assisted-by: Claude Opus 4.6
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2d5dbb37b2 (reftable/block: handle allocation failures, 2024-10-02)
taught `writer_reinit_block_writer()` to report initialization failures
and updated its callers, but `reftable_writer_new()` continued to ignore
the return value.
Consequently, the constructor could report success after block-writer
initialization had failed. Propagate the error and release the
constructor's allocations instead of returning an unusable writer.
Pointed out by GPT-5.6 Sol and Claude Opus 4.8.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
show_editor() calls launch_editor() to open the user's editor on
the configuration file, but discards the return value and
unconditionally returns 0 (success). When the editor fails to
launch (e.g., $EDITOR is not found, or the editor exits with a
nonzero status), the caller receives no indication that anything
went wrong.
This affects "git config edit" and "git config --edit": the
command silently succeeds even when the editor could not be
started. In contrast, other editor-launching paths in git (such
as "git commit" and "git rebase --edit-todo") properly propagate
editor failures and exit with an error.
Check the return value and propagate the failure by returning -1.
The two callers (cmd_config_edit at line 1315 and the legacy
cmd_config at line 1478) both propagate this return to
handle_builtin, which translates negative returns into an error
exit.
Pointed out by Coverity.
Assisted-by: Claude Opus 4.6
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
get_active_slot() duplicates the default curl handle via
curl_easy_duphandle() to create a per-slot session handle. The
return value is stored directly in slot->curl without checking
for NULL. curl_easy_duphandle() can return NULL when memory
allocation fails internally, and the libcurl documentation
explicitly states this possibility.
When this happens, slot->curl is NULL and the very next operation
(curl_easy_setopt on line 1632 for CURLOPT_COOKIEFILE) passes
NULL as the curl handle, which is undefined behavior in libcurl
and typically crashes.
Every HTTP operation in git goes through get_active_slot(), so
this affects all remote-https, remote-http, and HTTP-based
operations (clone, fetch, push over HTTP, bundle-uri downloads).
Add a NULL check and die() with a clear message. There is no
reasonable recovery from a failed handle duplication: the process
is out of memory and cannot perform any HTTP operation.
Pointed out by Coverity.
Assisted-by: Claude Opus 4.6
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
One of our tests for whether "maintenance.strategy" is being respected
in t7900 is flaky in our CI systems:
+ GIT_TRACE2_EVENT=/tmp/test-output/trash directory.t7900-maintenance/repo/trace2.txt git -c maintenance.strategy=incremental maintenance run --quiet
+ test_maintenance_tasks trace2.txt
+ cat
+ sed -ne s/.*"region_enter".*"category":"maintenance\([^"]*\)".*"label":"\([^"][^"]*\)".*/\2\1/p trace2.txt
+ test_cmp expect actual
+ test 2 -ne 2
+ eval /usr/bin/diff -u "$@"
+ /usr/bin/diff -u expect actual
--- expect 2026-08-07 06:20:51.388322602 +0000
+++ actual 2026-08-07 06:20:51.388322602 +0000
@@ -1,2 +0,0 @@
-gc foreground
-gc
When running with the "incremental" strategy, we expect two git-gc(1)
tasks to have been executed, but sometimes the test simply doesn't
execute any of those tasks.
A first hunch may be that maybe the disk-state is sometimes different
and thus we decide not to run maintenance. But git-maintenance(1)
doesn't run with the "--auto" switch, so we should execute those tasks
regardless of the on-disk state.
But there's a second condition that may cause us to not execute tasks,
namely when the "maintenance.lock" file exists due to a concurrently
running git-maintenance(1) process. We usually disable auto-maintenance
from detaching in our test suite to avoid exactly these kinds of race
conditions by exporting `GIT_TEST_MAINT_AUTO_DETACH=false`. But in t7900
we unset "GIT_TEST_MAINT_AUTO_DETACH" and thus enable the auto-detach
logic. The intent of this is to exercise git-maintenance(1) closer to
how it would run in a real-world scenario, but it does cause us to race
when the detached maintenance job that was triggered by `test_commit()`
lives long enough.
We could trivially fix this race by disabling auto-maintenance for this
specific test. But that doesn't fix this class of races in this test
suite: while I haven't seen any of the other tests fail in the same way,
a bunch of them have this race, as well.
Instead, let's retain "GIT_TEST_MAINT_AUTO_DETACH" and only unset it as
required.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano