git/commit.c, branch v2.48.2

Merge branch 'ps/build-sign-compare'

2024-12-23T17:32:11Z

Start working to make the codebase buildable with -Wsign-compare. * ps/build-sign-compare: t/helper: don't depend on implicit wraparound scalar: address -Wsign-compare warnings builtin/patch-id: fix type of `get_one_patchid()` builtin/blame: fix type of `length` variable when emitting object ID gpg-interface: address -Wsign-comparison warnings daemon: fix type of `max_connections` daemon: fix loops that have mismatching integer types global: trivial conversions to fix `-Wsign-compare` warnings pkt-line: fix -Wsign-compare warning on 32 bit platform csum-file: fix -Wsign-compare warning on 32-bit platform diff.h: fix index used to loop through unsigned integer config.mak.dev: drop `-Wno-sign-compare` global: mark code units that generate warnings with `-Wsign-compare` compat/win32: fix -Wsign-compare warning in "wWinMain()" compat/regex: explicitly ignore "-Wsign-compare" warnings git-compat-util: introduce macros to disable "-Wsign-compare" warnings

Merge branch 'bf/explicit-config-set-in-advice-messages'

2024-12-16T01:54:28Z

The advice messages now tell the newer 'git config set' command to set the advice.token configuration variable to squelch a message. * bf/explicit-config-set-in-advice-messages: advice: suggest using subcommand "git config set"

global: trivial conversions to fix `-Wsign-compare` warnings

2024-12-06T11:20:04Z

We have a bunch of loops which iterate up to an unsigned boundary using a signed index, which generates warnigs because we compare a signed and unsigned value in the loop condition. Address these sites for trivial cases and enable `-Wsign-compare` warnings for these code units. This patch only adapts those code units where we can drop the `DISABLE_SIGN_COMPARE_WARNINGS` macro in the same step. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

global: mark code units that generate warnings with `-Wsign-compare`

2024-12-06T11:20:02Z

Mark code units that generate warnings with `-Wsign-compare`. This allows for a structured approach to get rid of all such warnings over time in a way that can be easily measured. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

advice: suggest using subcommand "git config set"

2024-12-06T02:24:21Z

The advice message currently suggests using "git config advice..." to disable advice messages, but since 00bbdde141 (builtin/config: introduce "set" subcommand, 2024-05-06) we have the "set" subcommand for config. Since using the subcommand is more in-line with the modern interface, any advice should be promoting its usage. Change the disable advice message to use the subcommand instead. Change all uses of "git config advice" in the tests to use the subcommand. Signed-off-by: Bence Ferdinandy Signed-off-by: Junio C Hamano

commit: avoid leaking already-saved buffer

2024-09-25T17:24:53Z

When we parse a commit via repo_parse_commit_internal(), if save_commit_buffer is set we'll stuff the buffer of the object contents into a cache, overwriting any previous value. This can result in a leak of that previously cached value, though it's rare in practice. If we have a value in the cache it would have come from a previous parse, and during that parse we'd set the object.parsed flag, causing any subsequent parse attempts to exit without doing any work. But it's possible to "unparse" a commit, which we do when registering a commit graft. And since shallow fetches are implemented using grafts, the leak is triggered in practice by t5539. There are a number of possible ways to address this: 1. the unparsing function could clear the cached commit buffer, too. I think this would work for the case I found, but I'm not sure if there are other ways to end up in the same state (an unparsed commit with an entry in the commit buffer cache). 2. when we parse, we could check the buffer cache and prefer it to reading the contents from the object database. In theory the contents of a particular sha1 are immutable, but the code in question is violating the immutability with grafts. So this approach makes me a bit nervous, although I think it would work in practice (the grafts are applied to what we parse, but we still retain the original contents). 3. We could realize the cache is already populated and discard its contents before overwriting. It's possible some other code could be holding on to a pointer to the old cache entry (and we'd introduce a use-after-free), but I think the risk of that is relatively low. 4. The reverse of (3): when the cache is populated, don't bother saving our new copy. This is perhaps a little weird, since we'll have just populated the commit struct based on a different buffer. But the two buffers should be the same, even in the presence of grafts (as in (2) above). I went with option 4. It addresses the leak directly and doesn't carry any risk of breaking other assumptions. And it's the same technique used by parse_object_buffer() for this situation, though I'm not sure when it would even come up there. The extra safety has been there since bd1e17e245 (Make "parse_object()" also fill in commit message buffer data., 2005-05-25). This lets us mark t5539 as leak-free. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

Merge branch 'ps/environ-wo-the-repository'

2024-09-23T17:35:05Z

Code clean-up. * ps/environ-wo-the-repository: (21 commits) environment: stop storing "core.notesRef" globally environment: stop storing "core.warnAmbiguousRefs" globally environment: stop storing "core.preferSymlinkRefs" globally environment: stop storing "core.logAllRefUpdates" globally refs: stop modifying global `log_all_ref_updates` variable branch: stop modifying `log_all_ref_updates` variable repo-settings: track defaults close to `struct repo_settings` repo-settings: split out declarations into a standalone header environment: guard state depending on a repository environment: reorder header to split out `the_repository`-free section environment: move `set_git_dir()` and related into setup layer environment: make `get_git_namespace()` self-contained environment: move object database functions into object layer config: make dependency on repo in `read_early_config()` explicit config: document `read_early_config()` and `read_very_early_config()` environment: make `get_git_work_tree()` accept a repository environment: make `get_graft_file()` accept a repository environment: make `get_index_file()` accept a repository environment: make `get_object_directory()` accept a repository environment: make `get_git_common_dir()` accept a repository ...

Merge branch 'ps/leakfixes-part-6'

2024-09-20T18:16:30Z

More leakfixes. * ps/leakfixes-part-6: (22 commits) builtin/repack: fix leaking keep-pack list merge-ort: fix two leaks when handling directory rename modifications match-trees: fix leaking prefixes in `shift_tree()` builtin/fmt-merge-msg: fix leaking buffers builtin/grep: fix leaking object context builtin/pack-objects: plug leaking list of keep-packs builtin/repack: fix leaking line buffer when packing promisors negotiator/skipping: fix leaking commit entries shallow: fix leaking members of `struct shallow_info` shallow: free grafts when unregistering them object: clear grafts when clearing parsed object pool gpg-interface: fix misdesigned signing key interfaces send-pack: fix leaking push cert nonce remote: fix leak in reachability check of a remote-tracking ref remote: fix leaking tracking refs builtin/submodule--helper: fix leaking refs on push-check submodule: fix leaking fetch task data upload-pack: fix leaking child process data on reachability checks builtin/push: fix leaking refspec query result send-pack: fix leaking common object IDs ...

environment: make `get_graft_file()` accept a repository

2024-09-12T17:15:40Z

The `get_graft_file()` function retrieves the path to the graft file of `the_repository`. Make it accept a `struct repository` such that it can work on arbitrary repositories and make it part of the repository subsystem. This reduces our reliance on `the_repository` and clarifies scope. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

object: clear grafts when clearing parsed object pool

2024-09-05T15:49:11Z

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 Signed-off-by: Junio C Hamano