aboutsummaryrefslogtreecommitdiffstats
path: root/builtin/commit.c (follow)
AgeCommit message (Collapse)AuthorFilesLines
2024-07-08Merge branch 'ps/leakfixes-more'Junio C Hamano1-3/+7
More memory leaks have been plugged. * ps/leakfixes-more: (29 commits) builtin/blame: fix leaking ignore revs files builtin/blame: fix leaking prefixed paths blame: fix leaking data for blame scoreboards line-range: plug leaking find functions merge: fix leaking merge bases builtin/merge: fix leaking `struct cmdnames` in `get_strategy()` sequencer: fix memory leaks in `make_script_with_merges()` builtin/clone: plug leaking HEAD ref in `wanted_peer_refs()` apply: fix leaking string in `match_fragment()` sequencer: fix leaking string buffer in `commit_staged_changes()` commit: fix leaking parents when calling `commit_tree_extended()` config: fix leaking "core.notesref" variable rerere: fix various trivial leaks builtin/stash: fix leak in `show_stash()` revision: free diff options builtin/log: fix leaking commit list in git-cherry(1) merge-recursive: fix memory leak when finalizing merge builtin/merge-recursive: fix leaking object ID bases builtin/difftool: plug memory leaks in `run_dir_diff()` object-name: free leaking object contexts ...
2024-06-11commit: fix leaking parents when calling `commit_tree_extended()`Patrick Steinhardt1-1/+2
When creating commits via `commit_tree_extended()`, the caller passes in a string list of parents. This call implicitly transfers ownership of that list to the function, which is quite surprising to begin with. But to make matters worse, `commit_tree_extended()` doesn't even bother to free the list of parents in error cases. The result is a memory leak, and one that the caller cannot fix by themselves because they do not know whether parts of the string list have already been released. Refactor the code such that callers can keep ownership of the list of parents, which is getting indicated by parameter being a constant pointer now. Free the lists at the calling site and add a common exit path to those sites as required. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-11parse-options: fix leaks for users of OPT_FILENAMEPatrick Steinhardt1-2/+5
The `OPT_FILENAME()` option will, if set, put an allocated string into the user-provided variable. Consequently, that variable thus needs to be free'd by the caller of `parse_options()`. Some callsites don't though and thus leak memory. Fix those. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07global: improve const correctness when assigning string constantsPatrick Steinhardt1-3/+3
We're about to enable `-Wwrite-strings`, which changes the type of string constants to `const char[]`. Fix various sites where we assign such constants to non-const variables. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-27config: clarify memory ownership in `git_config_string()`Patrick Steinhardt1-1/+1
The out parameter of `git_config_string()` is a `const char **` even though we transfer ownership of memory to the caller. This is quite misleading and has led to many memory leaks all over the place. Adapt the parameter to instead be `char **`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-27config: clarify memory ownership in `git_config_pathname()`Patrick Steinhardt1-1/+1
The out parameter of `git_config_pathname()` is a `const char **` even though we transfer ownership of memory to the caller. This is quite misleading and has led to many memory leaks all over the place. Adapt the parameter to instead be `char **`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-15Merge branch 'jp/tag-trailer'Junio C Hamano1-17/+3
"git tag" learned the "--trailer" option to futz with the trailers in the same way as "git commit" does. * jp/tag-trailer: builtin/tag: add --trailer option builtin/commit: refactor --trailer logic builtin/commit: use ARGV macro to collect trailers
2024-05-07builtin/commit: refactor --trailer logicJohn Passaro1-8/+2
git-commit adds user trailers to the commit message by passing its `--trailer` arguments to a child process running `git-interpret-trailers --in-place`. This logic is broadly useful, not just for git-commit but for other commands constructing message bodies (e.g. git-tag). Let's move this logic from git-commit to a new function in the trailer API, so that it can be re-used in other commands. Helped-by: Patrick Steinhardt <ps@pks.im> Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: John Passaro <john.a.passaro@gmail.com> Acked-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-07builtin/commit: use ARGV macro to collect trailersJohn Passaro1-9/+1
Replace git-commit's callback for --trailer with the standard OPT_PASSTHRU_ARGV macro. The callback only adds its values to a strvec and sanity-checks that `unset` is always false; both of these are already implemented in the parse-option API. Signed-off-by: John Passaro <john.a.passaro@gmail.com> Acked-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-18builtin: stop using `the_index`Patrick Steinhardt1-41/+40
Convert builtins to use `the_repository->index` instead of `the_index`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-15Merge branch 'gt/add-u-commit-i-pathspec-check'Junio C Hamano1-1/+6
"git add -u <pathspec>" and "git commit [-i] <pathspec>" did not diagnose a pathspec element that did not match any files in certain situations, unlike "git add <pathspec>" did. * gt/add-u-commit-i-pathspec-check: builtin/add: error out when passing untracked path with -u builtin/commit: error out when passing untracked path with -i revision: optionally record matches with pathspec elements
2024-04-05Merge branch 'jk/core-comment-string'Junio C Hamano1-10/+11
core.commentChar used to be limited to a single byte, but has been updated to allow an arbitrary multi-byte sequence. * jk/core-comment-string: config: add core.commentString config: allow multi-byte core.commentChar environment: drop comment_line_char compatibility macro wt-status: drop custom comment-char stringification sequencer: handle multi-byte comment characters when writing todo list find multi-byte comment chars in unterminated buffers find multi-byte comment chars in NUL-terminated strings prefer comment_line_str to comment_line_char for printing strbuf: accept a comment string for strbuf_add_commented_lines() strbuf: accept a comment string for strbuf_commented_addf() strbuf: accept a comment string for strbuf_stripspace() environment: store comment_line_char as a string strbuf: avoid shadowing global comment_line_char name commit: refactor base-case of adjust_comment_line_char() strbuf: avoid static variables in strbuf_add_commented_lines() strbuf: simplify comment-handling in add_lines() helper config: forbid newline as core.commentChar
2024-04-03builtin/commit: error out when passing untracked path with -iGhanshyam Thakkar1-1/+6
When we provide a pathspec which does not match any tracked path alongside --include, we do not error like without --include. If there is something staged, it will commit the staged changes and ignore the pathspec which does not match any tracked path. And if nothing is staged, it will print the status. Exit code is 0 in both cases (unlike without --include). This is also described in the TODO comment before the relevant testcase. Fix this by passing a character array to add_files_to_cache() to collect the pathspec matching information and error out if the given path is untracked. Also, amend the testcase to check for the error message and remove the TODO comment. Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-03revision: optionally record matches with pathspec elementsJunio C Hamano1-1/+1
Unlike "git add" and other end-user facing commands, where it is diagnosed as an error to give a pathspec with an element that does not match any path, the diff machinery does not care if some elements of the pathspec do not match. Given that the diff machinery is heavily used in pathspec-limited "git log" machinery, and it is common for a path to come and go while traversing the project history, this is usually a good thing. However, in some cases we would want to know if all the pathspec elements matched. For example, "git add -u <pathspec>" internally uses the machinery used by "git diff-files" to decide contents from what paths to add to the index, and as an end-user facing command, "git add -u" would want to report an unmatched pathspec element. Add a new .ps_matched member next to the .prune_data member in "struct rev_info" so that we can optionally keep track of the use of .prune_data pathspec elements that can be inspected by the caller. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-28Merge branch 'jc/show-untracked-false'Junio C Hamano1-19/+38
The status.showUntrackedFiles configuration variable had a name that tempts users to set a Boolean value expressed in our usual "false", "off", and "0", but it only took "no". This has been corrected so "true" and its synonyms are taken as "normal", while "false" and its synonyms are taken as "no". * jc/show-untracked-false: status: allow --untracked=false and friends status: unify parsing of --untracked= and status.showUntrackedFiles
2024-03-28Merge branch 'eb/hash-transition'Junio C Hamano1-1/+1
Work to support a repository that work with both SHA-1 and SHA-256 hash algorithms has started. * eb/hash-transition: (30 commits) t1016-compatObjectFormat: add tests to verify the conversion between objects t1006: test oid compatibility with cat-file t1006: rename sha1 to oid test-lib: compute the compatibility hash so tests may use it builtin/ls-tree: let the oid determine the output algorithm object-file: handle compat objects in check_object_signature tree-walk: init_tree_desc take an oid to get the hash algorithm builtin/cat-file: let the oid determine the output algorithm rev-parse: add an --output-object-format parameter repository: implement extensions.compatObjectFormat object-file: update object_info_extended to reencode objects object-file-convert: convert commits that embed signed tags object-file-convert: convert commit objects when writing object-file-convert: don't leak when converting tag objects object-file-convert: convert tag objects when writing object-file-convert: add a function to convert trees between algorithms object: factor out parse_mode out of fast-import and tree-walk into in object.h cache: add a function to read an OID of a specific algorithm tag: sign both hashes commit: export add_header_signature to support handling signatures on tags ...
2024-03-13status: allow --untracked=false and friendsJunio C Hamano1-2/+11
It is natural to expect that the "--untracked" option and the status.showuntrackedFiles configuration variable to take a Boolean value ("do you want me to show untracked files?"), but the current code takes nothing but "no" as "no, please do not show any". Allow the usual Boolean values to be given, and treat 'true' as "normal", and 'false' as "no". Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-13status: unify parsing of --untracked= and status.showUntrackedFilesJunio C Hamano1-17/+27
There are two code paths that take a string and parse it to enum untracked_status_type. Introduce a helper function and use it. As these two places handle an error differently, add an additional invalid value to the enum, and have the caller of the helper handle the error condition, instead of dying or emitting error message from the helper. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12prefer comment_line_str to comment_line_char for printingJeff King1-6/+6
As part of our transition to multi-byte comment characters, we should use the string variable rather than the historical character variable. All of the sites adjusted here are just swapping out "%c" for "%s" in format strings, or strbuf_addch() for strbuf_addstr(). The type system and printf-attribute give the compiler enough information to make sure our formats and variable changes all match (especially important for cases where the format string is defined far away from its use, like prepare_to_commit() in commit.c). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12strbuf: accept a comment string for strbuf_stripspace()Jeff King1-1/+1
As part of our transition to multi-byte comment characters, let's take a NUL-terminated string pointer for strbuf_stripspace(), rather than a single character. We can continue to support its feature of ignoring comments by accepting a NULL pointer (as opposed to the current behavior of a NUL byte). All of the callers have to be adjusted, but they can all just pass comment_line_str (or NULL). Inside the function we detect comments by comparing the first byte of a line to the comment character. We'll adjust that to use starts_with(), which will match multiple bytes (though for now, of course, we still only allow a single byte, so it's academic). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12environment: store comment_line_char as a stringJeff King1-2/+2
We'd like to eventually support multi-byte comment prefixes, but the comment_line_char variable is referenced in many spots, making the transition difficult. Let's start by storing the character in a NUL-terminated string. That will let us switch code over incrementally to the string format, and we can easily support the existing code with a macro wrapper (since we'll continue to allow only a single-byte prefix, this will behave identically). Once all references to the "char" variable have been converted, we can drop it and enable longer strings. We'll still have to touch all of the spots that create or set the variable in this patch, but there are only a few (reading the config, and the "auto" character selector). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12commit: refactor base-case of adjust_comment_line_char()Jeff King1-2/+3
When core.commentChar is set to "auto", we check a set of candidate characters against the proposed buffer to see which if any can be used without ambiguity. But before we do that, we optimize for the common case that the default "#" is fine by just seeing if it is present in the buffer at all. The way we do this is a bit subtle, though: we assign the candidate character to comment_line_char preemptively, then check if it works, and return if it does. The subtle part is that sometimes setting comment_line_char is important (after we return, the important outcome is the fact that we have set the variable) and sometimes it is useless (if our optimization fails, we go on to do the more careful checks and eventually assign something else instead). To make it more clear what is happening (and to make further refactoring of comment_line_char easier), let's check our candidate character directly, and then assign as part of returning if it worked out. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-07Merge branch 'jt/commit-redundant-scissors-fix'Junio C Hamano1-7/+5
"git commit -v --cleanup=scissors" used to add the scissors line twice in the log message buffer, which has been corrected. * jt/commit-redundant-scissors-fix: commit: unify logic to avoid multiple scissors lines when merging commit: avoid redundant scissor line with --cleanup=scissors -v
2024-03-07Merge branch 'js/merge-tree-3-trees'Junio C Hamano1-1/+2
"git merge-tree" has learned that the three trees involved in the 3-way merge only need to be trees, not necessarily commits. * js/merge-tree-3-trees: fill_tree_descriptor(): mark error message for translation cache-tree: avoid an unnecessary check Always check `parse_tree*()`'s return value t4301: verify that merge-tree fails on missing blob objects merge-ort: do check `parse_tree()`'s return value merge-tree: fail with a non-zero exit code on missing tree objects merge-tree: accept 3 trees as arguments
2024-02-27commit: unify logic to avoid multiple scissors lines when mergingJosh Triplett1-5/+3
prepare_to_commit has some logic to figure out whether merge already added a scissors line, and therefore it shouldn't add another. Now that wt_status_add_cut_line has built-in state for whether it has already added a previous line, just set that state instead, and then remove that condition from subsequent calls to wt_status_add_cut_line. Signed-off-by: Josh Triplett <josh@joshtriplett.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-02-27commit: avoid redundant scissor line with --cleanup=scissors -vJosh Triplett1-2/+2
`git commit --cleanup=scissors -v` prints two scissors lines: one at the start of the comment lines, and the other right before the diff. This is redundant, and pushes the diff further down in the user's editor than it needs to be. Make wt_status_add_cut_line() remember if it has added a cut line before, and avoid adding a redundant one. Add a test for this. Signed-off-by: Josh Triplett <josh@joshtriplett.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-02-23Always check `parse_tree*()`'s return valueJohannes Schindelin1-1/+2
Otherwise we may easily run into serious crashes: For example, if we run `init_tree_desc()` directly after a failed `parse_tree()`, we are accessing uninitialized data or trying to dereference `NULL`. Note that the `parse_tree()` function already takes care of showing an error message. The `parse_tree_indirectly()` and `repo_get_commit_tree()` functions do not, therefore those latter call sites need to show a useful error message while the former do not. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-02-08Merge branch 'en/header-cleanup' into maint-2.43Junio C Hamano1-8/+0
Remove unused header "#include". * en/header-cleanup: treewide: remove unnecessary includes in source files treewide: add direct includes currently only pulled in transitively trace2/tr2_tls.h: remove unnecessary include submodule-config.h: remove unnecessary include pkt-line.h: remove unnecessary include line-log.h: remove unnecessary include http.h: remove unnecessary include fsmonitor--daemon.h: remove unnecessary includes blame.h: remove unnecessary includes archive.h: remove unnecessary include treewide: remove unnecessary includes in source files treewide: remove unnecessary includes from header files
2024-02-08Merge branch 'la/trailer-cleanups' into maint-2.43Junio C Hamano1-1/+1
Code clean-up. * la/trailer-cleanups: trailer: use offsets for trailer_start/trailer_end trailer: find the end of the log message commit: ignore_non_trailer computes number of bytes to ignore
2024-01-19refs: convert MERGE_AUTOSTASH to become a normal pseudo-refPatrick Steinhardt1-1/+1
Similar to the preceding conversion of the AUTO_MERGE pseudo-ref, let's convert the MERGE_AUTOSTASH ref to become a normal pseudo-ref as well. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-01-08Merge branch 'en/header-cleanup'Junio C Hamano1-8/+0
Remove unused header "#include". * en/header-cleanup: treewide: remove unnecessary includes in source files treewide: add direct includes currently only pulled in transitively trace2/tr2_tls.h: remove unnecessary include submodule-config.h: remove unnecessary include pkt-line.h: remove unnecessary include line-log.h: remove unnecessary include http.h: remove unnecessary include fsmonitor--daemon.h: remove unnecessary includes blame.h: remove unnecessary includes archive.h: remove unnecessary include treewide: remove unnecessary includes in source files treewide: remove unnecessary includes from header files
2024-01-02Merge branch 'la/trailer-cleanups'Junio C Hamano1-1/+1
Code clean-up. * la/trailer-cleanups: trailer: use offsets for trailer_start/trailer_end trailer: find the end of the log message commit: ignore_non_trailer computes number of bytes to ignore
2023-12-26treewide: remove unnecessary includes in source filesElijah Newren1-8/+0
Each of these were checked with gcc -E -I. ${SOURCE_FILE} | grep ${HEADER_FILE} to ensure that removing the direct inclusion of the header actually resulted in that header no longer being included at all (i.e. that no other header pulled it in transitively). ...except for a few cases where we verified that although the header was brought in transitively, nothing from it was directly used in that source file. These cases were: * builtin/credential-cache.c * builtin/pull.c * builtin/send-pack.c Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-10-30Merge branch 'jc/commit-new-underscore-index-fix'Junio C Hamano1-4/+4
Message fix. * jc/commit-new-underscore-index-fix: commit: do not use cryptic "new_index" in end-user facing messages
2023-10-20commit: ignore_non_trailer computes number of bytes to ignoreLinus Arver1-1/+1
ignore_non_trailer() returns the _number of bytes_ that should be ignored from the end of the log message. It does not by itself "ignore" anything. Rename this function to remove the leading "ignore" verb, to sound more like a quantity than an action. Signed-off-by: Linus Arver <linusa@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-10-17commit: do not use cryptic "new_index" in end-user facing messagesJunio C Hamano1-4/+4
These error messages say "new_index" as if that spelling has some significance to the end users (e.g. the file "$GIT_DIR/new_index" has some issues), but that is not the case at all. The i18n folks were made to include the word literally in the translated messages, which was not a good idea at all. Spell it "new index", as we are just telling the users that we failed to create a new index file. The term is expected to be translated to the end-users' languages, not left as if it were a literal file name. This dates all the way back to the first re-implemenation of "git commit" command in C (the scripted version did not have such wording in its error messages), in f5bbc322 (Port git commit to C., 2007-11-08). Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-10-02tree-walk: init_tree_desc take an oid to get the hash algorithmEric W. Biederman1-1/+1
To make it possible for git ls-tree to display the tree encoded in the hash algorithm of the oid specified to git ls-tree, update init_tree_desc to take as a parameter the oid of the tree object. Update all callers of init_tree_desc and init_tree_desc_gently to pass the oid of the tree object. Use the oid of the tree object to discover the hash algorithm of the oid and store that hash algorithm in struct tree_desc. Use the hash algorithm in decode_tree_entry and update_tree_entry_internal to handle reading a tree object encoded in a hash algorithm that differs from the repositories hash algorithm. Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-08Merge branch 'js/empty-index-fixes'Junio C Hamano1-5/+2
A few places failed to differenciate the case where the index is truly empty (nothing added) and we haven't yet read from the on-disk index file, which have been corrected. * js/empty-index-fixes: commit -a -m: allow the top-level tree to become empty again split-index: accept that a base index can be empty do_read_index(): always mark index as initialized unless erroring out
2023-07-06Merge branch 'gc/config-context'Junio C Hamano1-8/+12
Reduce reliance on a global state in the config reading API. * gc/config-context: config: pass source to config_parser_event_fn_t config: add kvi.path, use it to evaluate includes config.c: remove config_reader from configsets config: pass kvi to die_bad_number() trace2: plumb config kvi config.c: pass ctx with CLI config config: pass ctx with config files config.c: pass ctx in configsets config: add ctx arg to config_fn_t urlmatch.h: use config_fn_t type config: inline git_color_default_config
2023-07-06Merge branch 'cw/strbuf-cleanup'Junio C Hamano1-1/+1
Move functions that are not about pure string manipulation out of strbuf.[ch] * cw/strbuf-cleanup: strbuf: remove global variable path: move related function to path object-name: move related functions to object-name credential-store: move related functions to credential-store file abspath: move related functions to abspath strbuf: clarify dependency strbuf: clarify API boundary
2023-06-29Merge branch 'en/header-split-cache-h-part-3'Junio C Hamano1-3/+7
Header files cleanup. * en/header-split-cache-h-part-3: (28 commits) fsmonitor-ll.h: split this header out of fsmonitor.h hash-ll, hashmap: move oidhash() to hash-ll object-store-ll.h: split this header out of object-store.h khash: name the structs that khash declares merge-ll: rename from ll-merge git-compat-util.h: remove unneccessary include of wildmatch.h builtin.h: remove unneccessary includes list-objects-filter-options.h: remove unneccessary include diff.h: remove unnecessary include of oidset.h repository: remove unnecessary include of path.h log-tree: replace include of revision.h with simple forward declaration cache.h: remove this no-longer-used header read-cache*.h: move declarations for read-cache.c functions from cache.h repository.h: move declaration of the_index from cache.h merge.h: move declarations for merge.c from cache.h diff.h: move declaration for global in diff.c from cache.h preload-index.h: move declarations for preload-index.c from elsewhere sparse-index.h: move declarations for sparse-index.c from cache.h name-hash.h: move declarations for name-hash.c from cache.h run-command.h: move declarations for run-command.c from cache.h ...
2023-06-29commit -a -m: allow the top-level tree to become empty againJohannes Schindelin1-5/+2
In 03267e8656c (commit: discard partial cache before (re-)reading it, 2022-11-08), a memory leak was plugged by discarding any partial index before re-reading it. The problem with this memory leak fix is that it was based on an incomplete understanding of the logic introduced in 7168624c353 (Do not generate full commit log message if it is not going to be used, 2007-11-28). That logic was introduced to add a shortcut when committing without editing the commit message interactively. A part of that logic was to ensure that the index was read into memory: if (!active_nr && read_cache() < 0) die(...) Translation to English: If the index has not yet been read, read it, and if that fails, error out. That logic was incorrect, though: It used `!active_nr` as an indicator that the index was not yet read. Usually this is not a problem because in the vast majority of instances, the index contains at least one entry. And it was natural to do it this way because at the time that condition was introduced, the `index_state` structure had no explicit flag to indicate that it was initialized: This flag was only introduced in 913e0e99b6a (unpack_trees(): protect the handcrafted in-core index from read_cache(), 2008-08-23), but that commit did not adjust the code path where no index file was found and a new, pristine index was initialized. Now, when the index does not contain any entry (which is quite common in Git's test suite because it starts quite a many repositories from scratch), subsequent calls to `do_read_index()` will mistake the index not to be initialized, and read it again unnecessarily. This is a problem because after initializing the empty index e.g. the `cache_tree` in that index could have been initialized before a subsequent call to `do_read_index()` wants to ensure an initialized index. And if that subsequent call mistakes the index not to have been initialized, it would lead to leaked memory. The correct fix for that memory leak is to adjust the condition so that it does not mistake `active_nr == 0` to mean that the index has not yet been read. Using the `initialized` flag instead, we avoid that mistake, and as a bonus we can fix a bug at the same time that was introduced by the memory leak fix: When deleting all tracked files and then asking `git commit -a -m ...` to commit the result, Git would internally update the index, then discard and re-read the index undoing the update, and fail to commit anything. This fixes https://github.com/git-for-windows/git/issues/4462 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-28config: pass kvi to die_bad_number()Glen Choo1-4/+6
Plumb "struct key_value_info" through all code paths that end in die_bad_number(), which lets us remove the helper functions that read analogous values from "struct config_reader". As a result, nothing reads config_reader.config_kvi any more, so remove that too. In config.c, this requires changing the signature of git_configset_get_value() to 'return' "kvi" in an out parameter so that git_configset_get_<type>() can pass it to git_config_<type>(). Only numeric types will use "kvi", so for non-numeric types (e.g. git_configset_get_string()), pass NULL to indicate that the out parameter isn't needed. Outside of config.c, config callbacks now need to pass "ctx->kvi" to any of the git_config_<type>() functions that parse a config string into a number type. Included is a .cocci patch to make that refactor. The only exceptional case is builtin/config.c, where git_config_<type>() is called outside of a config callback (namely, on user-provided input), so config source information has never been available. In this case, die_bad_number() defaults to a generic, but perfectly descriptive message. Let's provide a safe, non-NULL for "kvi" anyway, but make sure not to change the message. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-28config: add ctx arg to config_fn_tGlen Choo1-4/+6
Add a new "const struct config_context *ctx" arg to config_fn_t to hold additional information about the config iteration operation. config_context has a "struct key_value_info kvi" member that holds metadata about the config source being read (e.g. what kind of config source it is, the filename, etc). In this series, we're only interested in .kvi, so we could have just used "struct key_value_info" as an arg, but config_context makes it possible to add/adjust members in the future without changing the config_fn_t signature. We could also consider other ways of organizing the args (e.g. moving the config name and value into config_context or key_value_info), but in my experiments, the incremental benefit doesn't justify the added complexity (e.g. a config_fn_t will sometimes invoke another config_fn_t but with a different config value). In subsequent commits, the .kvi member will replace the global "struct config_reader" in config.c, making config iteration a global-free operation. It requires much more work for the machinery to provide meaningful values of .kvi, so for now, merely change the signature and call sites, pass NULL as a placeholder value, and don't rely on the arg in any meaningful way. Most of the changes are performed by contrib/coccinelle/config_fn_ctx.pending.cocci, which, for every config_fn_t: - Modifies the signature to accept "const struct config_context *ctx" - Passes "ctx" to any inner config_fn_t, if needed - Adds UNUSED attributes to "ctx", if needed Most config_fn_t instances are easily identified by seeing if they are called by the various config functions. Most of the remaining ones are manually named in the .cocci patch. Manual cleanups are still needed, but the majority of it is trivial; it's either adjusting config_fn_t that the .cocci patch didn't catch, or adding forward declarations of "struct config_context ctx" to make the signatures make sense. The non-trivial changes are in cases where we are invoking a config_fn_t outside of config machinery, and we now need to decide what value of "ctx" to pass. These cases are: - trace2/tr2_cfg.c:tr2_cfg_set_fl() This is indirectly called by git_config_set() so that the trace2 machinery can notice the new config values and update its settings using the tr2 config parsing function, i.e. tr2_cfg_cb(). - builtin/checkout.c:checkout_main() This calls git_xmerge_config() as a shorthand for parsing a CLI arg. This might be worth refactoring away in the future, since git_xmerge_config() can call git_default_config(), which can do much more than just parsing. Handle them by creating a KVI_INIT macro that initializes "struct key_value_info" to a reasonable default, and use that to construct the "ctx" arg. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-26Merge branch 'jk/commit-use-no-divider-with-interpret-trailers'Junio C Hamano1-1/+2
When "git commit --trailer=..." invokes the interpret-trailers machinery, it knows what it feeds to interpret-trailers is a full log message without any patch, but failed to express that by passing the "--no-divider" option, which has been corrected. * jk/commit-use-no-divider-with-interpret-trailers: commit: pass --no-divider to interpret-trailers
2023-06-21repository: remove unnecessary include of path.hElijah Newren1-0/+1
This also made it clear that several .c files that depended upon path.h were missing a #include for it; add the missing includes while at it. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-21cache.h: remove this no-longer-used headerElijah Newren1-2/+1
Since this header showed up in some places besides just #include statements, update/clean-up/remove those other places as well. Note that compat/fsmonitor/fsm-path-utils-darwin.c previously got away with violating the rule that all files must start with an include of git-compat-util.h (or a short-list of alternate headers that happen to include it first). This change exposed the violation and caused it to stop building correctly; fix it by having it include git-compat-util.h first, as per policy. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-21read-cache*.h: move declarations for read-cache.c functions from cache.hElijah Newren1-0/+1
For the functions defined in read-cache.c, move their declarations from cache.h to a new header, read-cache-ll.h. Also move some related inline functions from cache.h to read-cache.h. The purpose of the read-cache-ll.h/read-cache.h split is that about 70% of the sites don't need the inline functions and the extra headers they include. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-21preload-index.h: move declarations for preload-index.c from elsewhereElijah Newren1-0/+1
We already have a preload-index.c file; move the declarations for the functions in that file into a new preload-index.h. These were previously split between cache.h and repository.h. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-21sparse-index.h: move declarations for sparse-index.c from cache.hElijah Newren1-0/+1
Note in particular that this reverses the decision made in 118a2e8bde0 ("cache: move ensure_full_index() to cache.h", 2021-04-01). Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>