git/send-pack.c, branch v2.47.2

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 ...

drop trailing newline from warning/error/die messages

2024-09-05T16:07:12Z

Our error reporting routines append a trailing newline, and the strings we pass to them should not include them (otherwise we get an extra blank line after the message). These cases were all found by looking at the results of: git grep -P '[^_](error|error_errno|warning|die|die_errno)\(.*\\n"[,)]' '*.c' Note that we _do_ sometimes include a newline in the middle of such messages, to create multiline output (hence our grep matching "," or ")" after we see the newline, so we know we're at the end of the string). It's possible that one or more of these cases could intentionally be including a blank line at the end, but having looked at them all manually, I think these are all just mistakes. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

gpg-interface: fix misdesigned signing key interfaces

2024-09-05T15:49:11Z

The interfaces to retrieve signing keys and their IDs are misdesigned as they return string constants even though they indeed allocate memory, which leads to memory leaks. Refactor the code to instead always return allocated strings and let the callers free them accordingly. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

send-pack: fix leaking push cert nonce

2024-09-05T15:49:11Z

When retrieving the push cert nonce from the server, we first store the constant returned by `server_feature_value()` and then, if the nonce is valid, we duplicate the nonce memory to a NUL-terminated string, so that we can pass it to `generate_push_cert()`. We never free the latter and thus cause a memory leak. Fix this by storing the limited-lifetime nonce into a scope-local variable such that the long-lived, allocated nonce can be easily freed without having to cast away its constness. This leak was exposed by t5534, but fixing it is not sufficient to make the whole test suite leak free. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

send-pack: fix leaking common object IDs

2024-09-05T15:49:10Z

We're leaking the array of common object IDs in `send_pack()`. Fix this by creating a common exit path where we free the leaking data. While at it, unify some other cleanups now that we have a central place to put them. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

send-pack: add new tracing regions for push

2024-08-22T22:02:32Z

At $DAYJOB we experienced some slow pushes and needed additional trace data to diagnose them. Add trace2 regions for various sections of send_pack(). Signed-off-by: Josh Steadmon Signed-off-by: Junio C Hamano

Merge branch 'jc/disable-push-nego-for-deletion'

2024-07-15T17:11:43Z

"git push" that pushes only deletion gave an unnecessary and harmless error message when push negotiation is configured, which has been corrected. * jc/disable-push-nego-for-deletion: push: avoid showing false negotiation errors

push: avoid showing false negotiation errors

2024-07-02T22:06:13Z

When "git push" is configured to use the push negotiation, a push of deletion of a branch (without pushing anything else) may end up not having anything to negotiate for the common ancestor discovery. In such a case, we end up making an internal invocation of "git fetch --negotiate-only" without any "--negotiate-tip" parameters that stops the negotiate-only fetch from being run, which by itself is not a bad thing (one fewer round-trip), but the end-user sees a "fatal: --negotiate-only needs one or more --negotiation-tip=*" message that the user cannot act upon. Teach "git push" to notice the situation and omit performing the negotiate-only fetch to begin with. One fewer process spawned, one fewer "alarming" message given the user. Signed-off-by: Junio C Hamano

Merge branch 'ps/use-the-repository'

2024-07-02T16:59:00Z

A CPP macro USE_THE_REPOSITORY_VARIABLE is introduced to help transition the codebase to rely less on the availability of the singleton the_repository instance. * ps/use-the-repository: hex: guard declarations with `USE_THE_REPOSITORY_VARIABLE` t/helper: remove dependency on `the_repository` in "proc-receive" t/helper: fix segfault in "oid-array" command without repository t/helper: use correct object hash in partial-clone helper compat/fsmonitor: fix socket path in networked SHA256 repos replace-object: use hash algorithm from passed-in repository protocol-caps: use hash algorithm from passed-in repository oidset: pass hash algorithm when parsing file http-fetch: don't crash when parsing packfile without a repo hash-ll: merge with "hash.h" refs: avoid include cycle with "repository.h" global: introduce `USE_THE_REPOSITORY_VARIABLE` macro hash: require hash algorithm in `empty_tree_oid_hex()` hash: require hash algorithm in `is_empty_{blob,tree}_oid()` hash: make `is_null_oid()` independent of `the_repository` hash: convert `oidcmp()` and `oideq()` to compare whole hash global: ensure that object IDs are always padded hash: require hash algorithm in `oidread()` and `oidclr()` hash: require hash algorithm in `hasheq()`, `hashcmp()` and `hashclr()` hash: drop (mostly) unused `is_empty_{blob,tree}_sha1()` functions

global: introduce `USE_THE_REPOSITORY_VARIABLE` macro

2024-06-14T17:26:33Z

Use of the `the_repository` variable is deprecated nowadays, and we slowly but steadily convert the codebase to not use it anymore. Instead, callers should be passing down the repository to work on via parameters. It is hard though to prove that a given code unit does not use this variable anymore. The most trivial case, merely demonstrating that there is no direct use of `the_repository`, is already a bit of a pain during code reviews as the reviewer needs to manually verify claims made by the patch author. The bigger problem though is that we have many interfaces that implicitly rely on `the_repository`. Introduce a new `USE_THE_REPOSITORY_VARIABLE` macro that allows code units to opt into usage of `the_repository`. The intent of this macro is to demonstrate that a certain code unit does not use this variable anymore, and to keep it from new dependencies on it in future changes, be it explicit or implicit For now, the macro only guards `the_repository` itself as well as `the_hash_algo`. There are many more known interfaces where we have an implicit dependency on `the_repository`, but those are not guarded at the current point in time. Over time though, we should start to add guards as required (or even better, just remove them). Define the macro as required in our code units. As expected, most of our code still relies on the global variable. Nearly all of our builtins rely on the variable as there is no way yet to pass `the_repository` to their entry point. For now, declare the macro in "biultin.h" to keep the required changes at least a little bit more contained. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano