git/diff.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

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

packfile: pass down repository to `has_object[_kept]_pack`

2024-12-03T23:21:54Z

The functions `has_object[_kept]_pack` currently rely on the global variable `the_repository`. To eliminate global variable usage in `packfile.c`, we should progressively shift the dependency on the_repository to higher layers. Let's remove its usage from these functions and any related ones. Signed-off-by: Karthik Nayak Signed-off-by: Junio C Hamano

Merge branch 'jk/output-prefix-cleanup'

2024-10-10T21:22:30Z

Code clean-up. * jk/output-prefix-cleanup: diff: store graph prefix buf in git_graph struct diff: return line_prefix directly when possible diff: return const char from output_prefix callback diff: drop line_prefix_length field line-log: use diff_line_prefix() instead of custom helper

diff: return const char from output_prefix callback

2024-10-03T21:22:22Z

The diff_options structure has an output_prefix callback for returning a prefix string, but it does so by returning a pointer to a strbuf. This makes the interface awkward. There's no reason the callback should need to use a strbuf, and it creates questions about whether the ownership of the resulting buffer should be transferred to the caller (it should not be, but a recent attempt to clean up this code led to a double-free in some cases). The one advantage we get is that the strbuf contains a ptr/len pair, so we could in theory have a prefix with embedded NULs. But we can observe that none of the existing callbacks would ever produce such a NUL (they are usually just indentation or graph symbols, and even the "--line-prefix" option takes a NUL-terminated string). And anyway, only one caller (the one in log_tree_diff_flush) actually looks at the strbuf length. In every other case we use a helper function which discards the length and just returns the NUL-terminated string. So let's just have the callback return a "const char *" pointer. It's up to the callbacks themselves if they want to use a strbuf under the hood. And now the caller in log_tree_diff_flush() can just use the helper function along with everybody else. That lets us even simplify out the function pointer check, since the helper returns an empty string (technically this does mean we'll sometimes issue an empty fputs() call, but I don't think this code path is hot enough to care about that). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

diff: drop line_prefix_length field

2024-10-03T21:22:21Z

The diff_options structure holds a line_prefix string and an associated length. But the length is always just the strlen() of the NUL-terminated string. Let's simplify the code by just storing the string pointer and assuming it is NUL-terminated when we use it. This will cause us to compute the string length in a few extra spots, but I don't think any of these are particularly hot code paths. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

diff: improve lifecycle management of diff queues

2024-09-30T18:23:05Z

The lifecycle management of diff queues is somewhat confusing: - For most of the part this can be attributed to `DIFF_QUEUE_CLEAR()`, which does not release any memory but rather initializes the queue, only. This is in contrast to our common naming schema, where "clearing" means that we release underlying memory and then re-initialize the data structure such that it is ready to use. - A second offender is `diff_free_queue()`, which does not free the queue structure itself. It is rather a release-style function. Refactor the code to make things less confusing. `DIFF_QUEUE_CLEAR()` is replaced by `DIFF_QUEUE_INIT` and `diff_queue_init()`, while `diff_free_queue()` is replaced by `diff_queue_release()`. While on it, adapt callsites where we call `DIFF_QUEUE_CLEAR()` with the intent to release underlying memory to instead call `diff_queue_clear()` to fix memory leaks. This memory leak is exposed by t4211, but plugging it alone does not make the whole test suite pass. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

Merge branch 'ps/leakfixes-part-7' into ps/leakfixes-part-8

2024-09-30T18:22:10Z

* ps/leakfixes-part-7: (23 commits) diffcore-break: fix leaking filespecs when merging broken pairs revision: fix leaking parents when simplifying commits builtin/maintenance: fix leak in `get_schedule_cmd()` builtin/maintenance: fix leaking config string promisor-remote: fix leaking partial clone filter grep: fix leaking grep pattern submodule: fix leaking submodule ODB paths trace2: destroy context stored in thread-local storage builtin/difftool: plug several trivial memory leaks builtin/repack: fix leaking configuration diffcore-order: fix leaking buffer when parsing orderfiles parse-options: free previous value of `OPTION_FILENAME` diff: fix leaking orderfile option builtin/pull: fix leaking "ff" option dir: fix off by one errors for ignored and untracked entries builtin/submodule--helper: fix leaking remote ref on errors t/helper: fix leaking subrepo in nested submodule config helper builtin/submodule--helper: fix leaking error buffer builtin/submodule--helper: clear child process when not running it submodule: fix leaking update strategy ...

diff: fix leaking orderfile option

2024-09-27T15:25:35Z

The `orderfile` diff option is being assigned via `OPT_FILENAME()`, which assigns an allocated string to the variable. We never free it though, causing a memory leak. Change the type of the string to `char *` and free it to plug the leak. This also requires us to use `xstrdup()` to assign the global config to it in case it is set. This leak is being hit in t7621, but plugging it alone does not make the test suite pass. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

Merge branch 'rs/diff-exit-code-binary'

2024-09-26T01:24:52Z

"git diff --exit-code" ignored modified binary files, which has been corrected. * rs/diff-exit-code-binary: diff: report modified binary files as changes in builtin_diff()