git/diff.h, branch v2.47.2

Merge branch 'jk/output-prefix-cleanup' into maint-2.47

2024-11-25T03:29:39Z

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

Merge branch 'ps/leakfixes-part-7'

2024-10-02T14:46:26Z

More leak-fixes. * 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 'jc/range-diff-lazy-setup'

2024-09-16T21:22:55Z

Code clean-up. * jc/range-diff-lazy-setup: remerge-diff: clean up temporary objdir at a central place remerge-diff: lazily prepare temporary objdir on demand

remerge-diff: clean up temporary objdir at a central place

2024-08-09T22:42:40Z

After running a diff between two things, or a series of diffs while walking the history, the diff computation is concluded by a call to diff_result_code() to extract the exit status of the diff machinery. The function can work on "struct diffopt", but all the callers historically and currently pass "struct diffopt" that is embedded in the "struct rev_info" that is used to hold the remerge_diff bit and the remerge_objdir variable that points at the temporary object directory in use. Redefine diff_result_code() to take the whole "struct rev_info" to give it an access to these members related to remerge-diff, so that it can get rid of the temporary object directory for any and all callers that used the feature. We can lose the equivalent code to do so from the code paths for individual commands, diff-tree, diff, and log. Signed-off-by: Junio C Hamano

hash-ll: merge with "hash.h"

2024-06-14T17:26:33Z

The "hash-ll.h" header was introduced via d1cbe1e6d8 (hash-ll.h: split out of hash.h to remove dependency on repository.h, 2023-04-22) to make explicit the split between hash-related functions that rely on the global `the_repository`, and those that don't. This split is no longer necessary now that we we have removed the reliance on `the_repository`. Merge "hash-ll.h" back into "hash.h". This causes some code units to not include "repository.h" anymore, which requires us to add some forward declarations. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

diff --stat: set the width defaults in a helper function

2023-09-29T22:46:06Z

Extract the commonly used initialization of the --stat-width=, --stat-name-width= and --stat-graph-with= parameters to their internal default values into a helper function, to avoid repeating the same initialization code in a few places. Add a couple of tests to additionally cover existing configuration options diff.statNameWidth= and diff.statGraphWidth= when used by git-merge to generate --stat outputs. This closes the gap that existed previously in the --stat tests, and reduces the chances for having any regressions introduced by this commit. While there, perform a small bunch of minor wording tweaks in the improved unit test, to improve its test-level consistency a bit. Signed-off-by: Dragan Simic Signed-off-by: Junio C Hamano

diff: drop useless "status" parameter from diff_result_code()

2023-08-21T22:33:24Z

Many programs use diff_result_code() to get a user-visible program exit code from a diff result (e.g., checking opts.found_changes if --exit-code was requested). This function also takes a "status" parameter, which seems at first glance that it could be used to propagate an error encountered when computing the diff. But it doesn't work that way: - negative values are passed through as-is, but are not appropriate as program exit codes - when --exit-code or --check is in effect, we _ignore_ the passed-in status completely. So a failed diff which did not have a chance to set opts.found_changes would erroneously report "success, no changes" instead of propagating the error. After recent cleanups, neither of these bugs is possible to trigger, as every caller just passes in "0". So rather than fixing them, we can simply drop the useless parameter instead. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano