git/editor.c, branch v2.40.2

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

run-command API: remove "env" member, always use "env_array"

2021-11-26T06:15:08Z

Remove the "env" member from "struct child_process" in favor of always using the "env_array". As with the preceding removal of "argv" in favor of "args" this gets rid of current and future oddities around memory management at the API boundary (see the amended API docs). For some of the conversions we can replace patterns like: child.env = env->v; With: strvec_pushv(&child.env_array, env->v); But for others we need to guard the strvec_pushv() with a NULL check, since we're not passing in the "v" member of a "struct strvec", e.g. in the case of tmp_objdir_env()'s return value. Ideally we'd rename the "env_array" member to simply "env" as a follow-up, since it and "args" are now inconsistent in not having an "_array" suffix, and seemingly without any good reason, unless we look at the history of how they came to be. But as we've currently got 122 in-tree hits for a "git grep env_array" let's leave that for now (and possibly forever). Doing that rename would be too disruptive. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

run-command API users: use strvec_pushl(), not argv construction

2021-11-26T06:15:07Z

Change a pattern of hardcoding an "argv" array size, populating it and assigning to the "argv" member of "struct child_process" to instead use "strvec_pushl()" to add data to the "args" member. This implements the same behavior as before in fewer lines of code, and moves us further towards being able to remove the "argv" member in a subsequent commit. Since we've entirely removed the "argv" variable(s) we can be sure that no potential logic errors of the type discussed in a preceding commit are being introduced here, i.e. ones where the local "argv" was being modified after the assignment to "struct child_process"'s "argv". Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

Merge branch 'jc/save-restore-terminal-revert'

2021-11-23T02:40:11Z

Regression fix for 2.34 * jc/save-restore-terminal-revert: Revert "editor: save and reset terminal after calling EDITOR"

Revert "editor: save and reset terminal after calling EDITOR"

2021-11-22T23:04:20Z

This reverts commit 3d411afabc9a96f41d47c07d6af6edda3d29ec92, blindly opening /dev/tty and calling tcsetattr() seems to be causing problems. cf. https://bugs.eclipse.org/bugs/show_bug.cgi?id=577358 cf. https://lore.kernel.org/git/04ab7301-ea34-476c-eae4-4044fef74b91@gmail.com/ Signed-off-by: Junio C Hamano

Merge branch 'cm/save-restore-terminal'

2021-10-18T22:47:57Z

An editor session launched during a Git operation (e.g. during 'git commit') can leave the terminal in a funny state. The code path has updated to save the terminal state before, and restore it after, it spawns an editor. * cm/save-restore-terminal: editor: save and reset terminal after calling EDITOR terminal: teach git how to save/restore its terminal settings

editor: save and reset terminal after calling EDITOR

2021-10-06T15:53:03Z

When EDITOR is invoked to modify a commit message, it will likely change the terminal settings, and if it misbehaves will leave the terminal output damaged as shown in a recent report from Windows Terminal[1] Instead use the functions provided by compat/terminal to save the settings and recover safely. [1] https://github.com/microsoft/terminal/issues/9359 Signed-off-by: Carlo Marcelo Arenas Belón Signed-off-by: Junio C Hamano

advice: remove read uses of most global `advice_` variables

2021-08-25T19:07:52Z

In c4a09cc9ccb (Merge branch 'hw/advise-ng', 2020-03-25), a new API for accessing advice variables was introduced and deprecated `advice_config` in favor of a new array, `advice_setting`. This patch ports all but two uses which read the status of the global `advice_` variables over to the new `advice_enabled` API. We'll deal with advice_add_embedded_repo and advice_graft_file_deprecated separately. Signed-off-by: Ben Boeckel Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

config: fix leaks from git_config_get_string_const()

2020-08-14T17:52:04Z

There are two functions to get a single config string: - git_config_get_string() - git_config_get_string_const() One might naively think that the first one allocates a new string and the second one just points us to the internal configset storage. But in fact they both allocate a new copy; the second one exists only to avoid having to cast when using it with a const global which we never intend to free. The documentation for the function explains that clearly, but it seems I'm not alone in being surprised by this. Of 17 calls to the function, 13 of them leak the resulting value. We could obviously fix these by adding the appropriate free(). But it would be simpler still if we actually had a non-allocating way to get the string. There's git_config_get_value() but that doesn't quite do what we want. If the config key is present but is a boolean with no value (e.g., "[foo]bar" in the file), then we'll get NULL (whereas the string versions will print an error and die). So let's introduce a new variant, git_config_get_string_tmp(), that behaves as these callers expect. We need a new name because we have new semantics but the same function signature (so even if we converted the four remaining callers, topics in flight might be surprised). The "tmp" is because this value should only be held onto for a short time. In practice it's rare for us to clear and refresh the configset, invalidating the pointer, but hopefully the "tmp" makes callers think about the lifetime. In each of the converted cases here the value only needs to last within the local function or its immediate caller. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

real_path: remove unsafe API

2020-03-10T18:41:40Z

Returning a shared buffer invites very subtle bugs due to reentrancy or multi-threading, as demonstrated by the previous patch. There was an unfinished effort to abolish this [1]. Let's finally rid of `real_path()`, using `strbuf_realpath()` instead. This patch uses a local `strbuf` for most places where `real_path()` was previously called. However, two places return the value of `real_path()` to the caller. For them, a `static` local `strbuf` was added, effectively pushing the problem one level higher: read_gitfile_gently() get_superproject_working_tree() [1] https://lore.kernel.org/git/1480964316-99305-1-git-send-email-bmwill@google.com/ Signed-off-by: Alexandr Miloslavskiy Signed-off-by: Junio C Hamano