git/builtin/worktree.c, branch v2.40.2

worktree: fix a trivial leak in prune_worktrees()

2023-02-06T23:34:38Z

We were leaking both the "struct strbuf" in prune_worktrees(), as well as the "path" we got from should_prune_worktree(). Since these were the only two uses of the "struct string_list" let's change it to a "DUP" and push these to it with "string_list_append_nodup()". For the string_list_append_nodup() we could also string_list_append() the main_path.buf, and then strbuf_release(&main_path) right away. But doing it this way avoids an allocation, as we already have the "struct strbuf" prepared for appending to "kept". Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

treewide: always have a valid "index_state.repo" member

2023-01-17T22:32:06Z

When the "repo" member was added to "the_index" in [1] the repo_read_index() was made to populate it, but the unpopulated "the_index" variable didn't get the same treatment. Let's do that in initialize_the_repository() when we set it up, and likewise for all of the current callers initialized an empty "struct index_state". This simplifies code that needs to deal with "the_index" or a custom "struct index_state", we no longer need to second-guess this part of the "index_state" deep in the stack. A recent example of such second-guessing is the "istate->repo ? istate->repo : the_repository" code in [2]. We can now simply use "istate->repo". We're doing this by making use of the INDEX_STATE_INIT() macro (and corresponding function) added in [3], which now have mandatory "repo" arguments. Because we now call index_state_init() in repository.c's initialize_the_repository() we don't need to handle the case where we have a "repo->index" whose "repo" member doesn't match the "repo" we're setting up, i.e. the "Complete the double-reference" code in repo_read_index() being altered here. That logic was originally added in [1], and was working around the lack of what we now have in initialize_the_repository(). For "fsmonitor-settings.c" we can remove the initialization of a NULL "r" argument to "the_repository". This was added back in [4], and was needed at the time for callers that would pass us the "r" from an "istate->repo". Before this change such a change to "fsmonitor-settings.c" would segfault all over the test suite (e.g. in t0002-gitfile.sh). This change has wider eventual implications for "fsmonitor-settings.c". The reason the other lazy loading behavior in it is required (starting with "if (!r->settings.fsmonitor) ..." is because of the previously passed "r" being "NULL". I have other local changes on top of this which move its configuration reading to "prepare_repo_settings()" in "repo-settings.c", as we could now start to rely on it being called for our "r". But let's leave all of that for now, and narrowly remove this particular part of the lazy-loading. 1. 1fd9ae517c4 (repository: add repo reference to index_state, 2021-01-23) 2. ee1f0c242ef (read-cache: add index.skipHash config option, 2023-01-06) 3. 2f6b1eb794e (cache API: add a "INDEX_STATE_INIT" macro/function, add release_index(), 2023-01-12) 4. 1e0ea5c4316 (fsmonitor: config settings are repository-specific, 2022-03-25) Signed-off-by: Ævar Arnfjörð Bjarmason Acked-by: Derrick Stolee Signed-off-by: Junio C Hamano

cache API: add a "INDEX_STATE_INIT" macro/function, add release_index()

2023-01-16T18:46:58Z

Hopefully in some not so distant future, we'll get advantages from always initializing the "repo" member of the "struct index_state". To make that easier let's introduce an initialization macro & function. The various ad-hoc initialization of the structure can then be changed over to it, and we can remove the various "0" assignments in discard_index() in favor of calling index_state_init() at the end. While not strictly necessary, let's also change the CALLOC_ARRAY() of various "struct index_state *" to use an ALLOC_ARRAY() followed by index_state_init() instead. We're then adding the release_index() function and converting some callers (including some of these allocations) over to it if they either won't need to use their "struct index_state" again, or are just about to call index_state_init(). Signed-off-by: Ævar Arnfjörð Bjarmason Acked-by: Derrick Stolee Signed-off-by: Junio C Hamano

built-ins: use free() not UNLEAK() if trivial, rm dead code

2022-11-21T03:32:48Z

For a lot of uses of UNLEAK() it would be quite tricky to release the memory involved, or we're missing the relevant *_(release|clear)() functions. But in these cases we have them already, and can just invoke them on the variable(s) involved, instead of UNLEAK(). For "builtin/worktree.c" the UNLEAK() was also added in [1], but the struct member it's unleaking was removed in [2]. The only non-"int" member of that structure is "const char *keep_locked", which comes to us via "argv" or a string literal[3]. We have good visibility via the compiler and tooling (e.g. SANITIZE=address) on bad free()-ing, but none on UNLEAK() we don't need anymore. So let's prefer releasing the memory when it's easy. For "bugreport", "worktree" and "config" we need to start using a "ret = ..." return pattern. For "builtin/bugreport.c" these UNLEAK() were added in [4], and for "builtin/config.c" in [1]. For "config" the code seen here was the only user of the "value" variable. For "ACTION_{RENAME,REMOVE}_SECTION" we need to be sure to return the right exit code in the cases where we were relying on falling through to the top-level. I think there's still a use-case for UNLEAK(), but hat it's changed since then. Using it so that "we can see the real leaks" is counter-productive in these cases. It's more useful to have UNLEAK() be a marker of the remaining odd cases where it's hard to free() the memory for whatever reason. With this change less than 20 of them remain in-tree. 1. 0e5bba53af7 (add UNLEAK annotation for reducing leak false positives, 2017-09-08) 2. d861d34a6ed (worktree: remove extra members from struct add_opts, 2018-04-24) 3. 0db4961c49b (worktree: teach `add` to accept --reason with --lock, 2021-07-15) 4. 0e5bba53af7 and 00d8c311050 (commit: fix "author_ident" leak, 2022-05-12). Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Taylor Blau

doc txt & -h consistency: make "worktree" consistent

2022-10-13T16:32:58Z

Make the "worktree" -h output consistent with the *.txt version. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

worktree: define subcommand -h in terms of command -h

2022-10-13T16:32:58Z

Avoid repeating the "-h" output for the "git worktree" command, and instead define the usage of each subcommand with macros, so that the "-h" output for the command itself can re-use those definitions. See [1], [2] and [3] for prior art using the same pattern. 1. b25b727494f (builtin/multi-pack-index.c: define common usage with a macro, 2021-03-30) 2. 8757b35d443 (commit-graph: define common usage with a macro, 2021-08-23) 3. 1e91d3faf6c (reflog: move "usage" variables and use macros, 2022-03-17) Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

doc txt & -h consistency: fix mismatching labels

2022-10-13T16:32:56Z

Fix various inconsistencies between command SYNOPSIS and the corresponding -h output where our translatable labels didn't match up. In some cases we need to adjust the prose that follows the SYNOPSIS accordingly, as it refers back to the changed label. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

builtin/worktree.c: let parse-options parse subcommands

2022-08-19T18:13:16Z

'git worktree' parses its subcommands with a long list of if statements. parse-options has just learned to parse subcommands, so let's use that facility instead, with the benefits of shorter code, handling missing or unknown subcommands, and listing subcommands for Bash completion. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano

run-command API: rename "env_array" to "env"

2022-06-02T21:31:16Z

Start following-up on the rename mentioned in c7c4bdeccf3 (run-command API: remove "env" member, always use "env_array", 2021-11-25) of "env_array" to "env". The "env_array" name was picked in 19a583dc39e (run-command: add env_array, an optional argv_array for env, 2014-10-19) because "env" was taken. Let's not forever keep the oddity of "*_array" for this "struct strvec", but not for its "args" sibling. This commit is almost entirely made with a coccinelle rule[1]. The only manual change here is in run-command.h to rename the struct member itself and to change "env_array" to "env" in the CHILD_PROCESS_INIT initializer. The rest of this is all a result of applying [1]: * make contrib/coccinelle/run_command.cocci.patch * patch -p1 env_array + E->env I've avoided changing any comments and derived variable names here, that will all be done in the next commit. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

Merge branch 'pw/worktree-list-with-z'

2022-04-04T17:56:25Z

"git worktree list --porcelain" did not c-quote pathnames and lock reasons with unsafe bytes correctly, which is worked around by introducing NUL terminated output format with "-z". * pw/worktree-list-with-z: worktree: add -z option for list subcommand