There are no users of `odb_source_write_alternates()` in our tree
anymore. Remove that function and its supporting infrastructure.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When creating a repository with alternates we first initialize the
object database and then write alternates to it in a separate step. This
is unfortunate due to a couple of reasons:
- It requires us to have a `write_alternates()` callback, which is
unfortunate as we never even write alternates to an object database
after it has been created.
- We're about to make alternates an implementation detail of the
object database's backend in a future patch series, so having this
callback is suboptimal there.
- The backend has more flexibility with how exactly alternates are
configured when it itself is in full control over their setup at the
time where it creates the object database itself.
We have thus introduced the ability to write alternates right at
creation time in the preceding commits, and we have unified setup of
alternates into a single location. All that's left to do for us now is
to wire up alternates as an option for the database creation.
Do so.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add the ability to write alternates when creating the object database.
This change allows us to remove the `write_alternates()` callback in a
subsequent patch.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Similar as in the preceding commit, move the setup of alternates for
local clones with "--no-shared" into `collect_alternates()`. With this
step, the complete setup of alternates is now handled by that function.
Note that besides moving stuff around, it also fixes a bug: previously,
we did not know to resolve the referenced repository's common directory.
Consequently, when referencing a worktree we failed to resolve
alternates. But as `collect_alternates()` already knows to resolve the
commondir for "--local" we can simply reuse this resolved path for our
purpose.
Add two tests, the first one of which exercises this bug to avoid future
regressions. The second test ensures that we properly handle relative
alternates for a referenced worktree.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When cloning a local repository with "--shared" we add that repository
to the new repository's alternates. This is done in `clone_local()`,
which is responsible for performing local clones.
Move the logic into `collect_alternates()` to unify our setup of
alternates. Furthermore, this will allow us to set up alternates right
at creation time of the object database.
Note that the logic for cloning a local repository with "--no-shared" is
not yet part of `collect_alternates()`. This will be handled in the next
commit, but means that at this step, we may compute `commondir` without
it being used. It will become used in the next step though.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Users can pass "--reference{,-if-able}" to git-clone(1) to instruct it
to set up alternates for the newly created repository. This allows it to
reuse objects from the source repository so that in the best case we
don't have to clone all objects over.
Those options are handled by the confusingly named `setup_reference()`
function -- without the above context, one might rightfully believe that
it was about refs, not about alternates. The function itself is rather
simple: we loop through all provided alternate paths and then, if such
an alternate is valid, we write it to the object database.
In subsequent commits we're about to consolidate the complete setup of
alternates into this function, and furthermore we'll refactor the setup
of the object database to handle doing this for us instead of writing
the alternates into it one by one.
Prepare for this refactoring by collecting the alternates into a strvec.
Rename the function to `collect_alternates()` to clarify its scope, as
it does not set up the references itself anymore.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In a subsequent commit, `setup_reference()` will start to call
`copy_alternates()`. Prepare for this by moving the function further
down so that we can avoid adding a declaration.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When cloning a repository we defer initialization of the reference
database. This is because we don't yet know all details required for us
to initialize the refdb in the first place. Most importantly, what we
are missing is information about the object hash.
We don't do the same thing for the object database yet, but here we
essentially have the same problem. While the "files" database does not
need any information about the object format at creation time, alternate
backends are likely to require that information so that they can
properly set up their data structures.
Besides this forward-looking future proofing though, we also have a
second use case for deferring initialization of the object database,
namely alternates. When initializing the object database we do not yet
know whether we'll need alternates or not because this depends on the
repository we're about to clone from. If it is a local repository and
the user has passed "--reference{,-if-able}", then we will end up
writing alternates into the object database.
The ugly part though is that we cannot determine where the repository is
getting cloned from before it has been initialized. While we of course
already have access to the user-provided URI, that URI can be very well
rewritten via "url.<base>.insteadOf". We can of course read the global-
and system-level configuration to resolve it. But we explicitly resolve
the URI a second time after we have initialized the repository because
it can happen that we copy a ".git/config" over from our templates, and
that file may cause us to rewrite the path.
In a subsequent commit though we'll start to write alternates as part of
the repository initialization, so we'll need to have the URI properly
resolved before we can initialize the object database. This is ugly, but
as mentioned above it makes sense for us to defer its initialization
anyway so that we also know about the object hash already.
Defer creation of the object database until after we have resolved the
URI.
Note that this also requires us to defer the call to `setup_reference()`
until after we have created the object database. While you might think
that this function has something to do with references ("refs/*"), it is
in fact responsible for setting up alternates. Consequently, we can only
call it after we have created the object database already.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The function `init_db()` is responsible for creating the on-disk
directory structure required for a Git repository. It is used by both
git-init(1) and git-clone(1), and because their expected behaviour is
different we support a couple of flags:
- The `QUIET` flag controls whether the command is quiet or not. For
git-init(1) this is user-controllable, whereas for git-clone(1)
we're always quiet.
- The `EXIST_OK` flag controls whether a preexisting repository is
okay or not. For git-init(1) it is, for git-clone(1) it's not.
- The `SKIP_REFDB` flag controls whether the reference database should
already be created or not. For git-init(1) we do, but for
git-clone(1) we don't because it does not yet know about the default
branch and about the remote object hash.
Furthermore, we're about to add another divergence in behaviour, where
we have to also skip creation of the object database in git-clone(1).
This is becoming quite cumbersome though.
Instead of introducing another flag, start to split up concerns of the
function so that we never create the reference or object database. This
becomes the responsibility of the caller, which is thus free to defer
their creation to a later point in time. This lets us get rid of most of
the divergent behaviour:
- We don't need the `SKIP_REFDB` and a potential `SKIP_ODB` flags
anymore.
- We don't need the `QUIET` flag anymore, as nothing prints output
except for the final status message that tells the user that the
repository has been (re)initialized. But as this message is specific
to git-init(1), we can easily move it there.
The only piece of information we still have to convey is whether or not
reinitialization of a preexisting repository is okay. This is handled by
a new `reinit_ok` pointer that, if non-`NULL`, indicates that it is okay
to reinitialize the repository. Furthermore, the pointer will be written
to to indicate whether the repository was reinitialized or not, which we
need in git-init(1) to print the correct initialization message.
With these refactorings, `init_db()` is named quite misleadingly though,
as we don't create any of the reference or object databases anymore.
Rename it to `create_repository()`.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* ps/odb-eagerly-load-alternates:
odb: drop `alternates_db` field
odb: drop `loaded_alternates` field
odb: eagerly initialize alternates
odb: decouple source path comparisons from `the_repository`
setup: create ref and object databases after config is written
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
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
The 'git bisect' command has been taught a
'--reset-when-found[=<where>]' option that tells the command to
automatically run 'git bisect reset' to jump back to the original
state or to the found culprit.
* hn/bisect-reset-when-found:
bisect: add --reset-when-found to leave when done
bisect: let bisect_reset() optionally check out quietly
A compatibility wrapper for writev(3p) has been reintroduced,
including fixes for CMake build and 'MAX_IO_SIZE' limits on NonStop.
Calls to write(3p) in send_sideband() and cat_blob() have been
refactored to use writev(3p) wrappers to reduce syscall overhead.
* ps/writev:
fast-import: use writev(3p) to send cat-blob responses
sideband: use writev(3p) to send pktlines
wrapper: properly handle MAX_IO_SIZE in writev(3p)
wrapper: introduce writev(3p) wrappers
compat/posix: introduce writev(3p) wrapper
The known limitations of the ref format migration in 'git refs' have
been moved to be displayed as a warning admonition directly under the
description of the 'migrate' subcommand, improving visibility. A
reference to 'git-maintenance' has also been corrected to use the
'linkgit' macro.
* kh/doc-refs-migrate-limitations:
doc: refs: linkgit to git-maintenance(1)
doc: refs: put ref migration warning under the command
The `struct object_database::alternates_db` field tracks the value of
the "GIT_ALTERNATE_OBJECT_DIRECTORIES" environment variable and is
used in `odb_prepare_alternates()`. It's not necessary to store it as a
separate field anymore though, as we stopped lazy-loading alternates.
Consequently, we can simply pass it to `odb_prepare_alternates()` via
`odb_new()` now.
Do so and remove the field.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The `struct object_database::loaded_alternates` field tells us whether
or not alternates have been loaded already. This field was useful before
the preceding commit as we were indeed lazy-loading alternates. But now
that we started to eagerly load them we can assume them to be loaded
after `odb_new()`, and hence the field does not serve any purpose
anymore.
Remove it.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When creating the object database we initialize the main object database
source, but we don't yet initialize its alternates. Instead, we have
many calls to `odb_prepare_alternates()` cluttered around the code base
whenever we are about to iterate through the sources.
This lazy loading doesn't really add much value: the moment where we
read any object we _have_ to load the alternates anyway. So given that
most of our commands would access the object database this optimization
is not really buying us much in the first place. Quite on the contrary,
it makes the code harder to understand and is a potential source of bugs
in case any callsite forgot to prepare alternates before we iterate
through the sources.
Historically though there was a reason why we deferred lazy-loading: it
may happen that the repository has "core.ignoreCase" configured, and we
use that to deduplicate the list of alternates in case we had the same
alternate configured multiple times, but with different casing. We used
to initialize the object database before we had fully configured the
owning repository though, and consequently we couldn't access that
configuration yet. This has changed in the preceding commit though where
we started to parse "core.ignoreCase" manually.
Eagerly prepare alternates both when creating the object database and
when flushing its caches. Drop the now-unneeded calls to prepare the
alternates that are scattered across the code base.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When registering alternates we deduplicate object database sources by
their path so that the same source won't be added twice. Ever since
cf2dc1c238 (speed up alt_odb_usable() with many alternates, 2021-07-07)
this duplicate check is backed by a map keyed by the source's path,
using `fspathhash()` and `fspatheq()` as hash and equality functions,
respectively.
These functions are problematic in this context for two reasons:
- They implicitly depend on `the_repository` instead of the
repository that owns the object database.
- They derive case-sensitivity from `repo_ignore_case()`, which
returns a default value in case the repository's configuration has
not been parsed yet. Object database sources may be registered
before that is the case, so the answer may flip depending on when a
source gets registered.
Fix this by making the comparison self-contained in the object
database. Instead of using `fspathhash()` and `fspatheq()` we resolve
"core.ignoreCase" manually and then use the correct comparison function
based on the result. This requires us to migrate to a `struct hashmap`,
as the khash interface does not give us the ability to pass an arbitrary
payload to these functions, and hence we'd have to use global state to
decide which of those to use.
Note that we can unconditionally use `strihash()` to compute entry
hashes regardless of case sensitivity: a hash function only needs to
guarantee that equal keys have equal hashes, and a case-insensitive
hash satisfies this requirement for both case-sensitive and
case-insensitive equality.
Overall it's quite debatable whether all of this complexity really is
worth it, out of two reasons:
- We could linearly search through all sources to find duplicates. But
the mentioned commit cares about cases with thousands of alternates,
and a linear search would of course regress performance quite a bit.
This doesn't really feel like a reasonable case to care about, but I
don't feel comfortable regressing it anyway.
- It's dubious whether we should handle "core.ignoreCase" in the first
place. The downside would be that we might add the same alternate
multiple times with different casing. But this is an edge case, and
it's not even fully fixed because we don't resolve symlinks or
mountpoints, either.
So for now, keep this infrastructure in-place while removing the global
dependency on `the_repository`. We may want to revisit this in the
future though.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When creating a new repository we create both the reference and object
databases after we have finalized the repository. This ensures that
those subsystems find a fully-configured repository at the time where
they are asked to create their own on-disk data structures.
There is one exception though: while we have already fully configured
the repository at this point, we haven't yet written both
"core.sharedRepository" and "receive.denyNonFastforwards". The latter
configuration doesn't really matter to us, but the first one does as the
"files" object database source reads it.
This doesn't cause any problems right now, but it will in a subsequent
patch where we will start to read "core.ignoreCase" when creating the
object database. Move the initialization of both of these data
structures towards the end of `init_db()`. The only thing that now comes
after is status reporting, but that's it.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
'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
In commit 6623a528e0 (doc: clarify documentation for rename/copy
limits, 2021-07-15), the wording around rename limit options and config
variables were updated to point out that only the quadratic portion of
rename detection (or "exhaustive portion of rename/copy detection" as
used in that commit) was limited by these options, because exact rename
detection and basename-guided rename detection (which both run in time
linear in the number of files) still run before this limit is checked.
However, the short help message wasn't updated at the time; update it
too.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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>
Patrick Steinhardt