Various memory leak fixes

Hi,

this is the third version of my patch series that fixes various memory leaks throughout Git.

Changes compared to v2:

- Add a missing SOB.

- Fix some typos in commit messages.

- Explain more thoroughly why appendwholeline() may leak even on error.

- Move removal of const char ** cast to the correct commit.

Thanks!

Patrick

Patrick Steinhardt (21):
ci: add missing dependency for TTY prereq
t: mark a bunch of tests as leak-free
transport-helper: fix leaking helper name
strbuf: fix leak when appendwholeline() fails with EOF
checkout: clarify memory ownership in unique_tracking_name()
http: refactor code to clarify memory ownership
config: clarify memory ownership in git_config_pathname()
diff: refactor code to clarify memory ownership of prefixes
convert: refactor code to clarify ownership of
check_roundtrip_encoding
builtin/log: stop using globals for log config builtin/log: stop using globals for format config
config: clarify memory ownership in git_config_string()
config: plug various memory leaks
builtin/credential: clear credential before exit
commit-reach: fix memory leak in ahead_behind()
submodule: fix leaking memory for submodule entries
strvec: add functions to replace and remove strings builtin/mv: refactor add_slash() to always return allocated strings builtin/mv duplicate string list memory
builtin/mv: refactor to use struct strvec
builtin/mv: fix leaks for submodule gitfile paths

 Makefile                                      |   1 +
 alias.c                                       |   6 +-
 attr.c                                        |   2 +-
 attr.h                                        |   2 +-
 builtin/blame.c                               |   2 +-
 builtin/checkout.c                            |  14 +-
 builtin/commit.c                              |   4 +-
 builtin/config.c                              |   2 +-
 builtin/credential.c                          |   2 +
 builtin/log.c                                 | 708 ++++++++++--------
 builtin/merge.c                               |   4 +-
 builtin/mv.c                                  | 222 +++---
 builtin/rebase.c                              |   2 +-
 builtin/receive-pack.c                        |   6 +-
 builtin/repack.c                              |   8 +-
 builtin/worktree.c                            |  20 +-
 checkout.c                                    |   4 +-
 checkout.h                                    |   6 +-
 ci/install-dependencies.sh                    |   4 +-
 commit-reach.c                                |   4 +
 config.c                                      |  52 +-
 config.h                                      |  10 +-
 convert.c                                     |  30 +-
 convert.h                                     |   2 +-
 delta-islands.c                               |   2 +-
 diff.c                                        |  20 +-
 environment.c                                 |  16 +-
 environment.h                                 |  14 +-
 fetch-pack.c                                  |   4 +-
 fsck.c                                        |   4 +-
 fsmonitor-settings.c                          |   5 +-
 gpg-interface.c                               |   6 +-
 http.c                                        |  50 +-
 imap-send.c                                   |  12 +-
 mailmap.c                                     |   4 +-
 mailmap.h                                     |   4 +-
 merge-ll.c                                    |   6 +-
 pager.c                                       |   2 +-
 pretty.c                                      |  14 +-
 promisor-remote.h                             |   2 +-
 remote.c                                      |  20 +-
 remote.h                                      |   8 +-
 sequencer.c                                   |   2 +-
 setup.c                                       |   6 +-
 strbuf.c                                      |   4 +-
 strvec.c                                      |  20 +
 strvec.h                                      |  13 +
 submodule-config.c                            |   2 +
 t/t0300-credentials.sh                        |   2 +
 t/t0411-clone-from-partial.sh                 |   1 +
 t/t0610-reftable-basics.sh                    |   1 +
 t/t0611-reftable-httpd.sh                     |   1 +
 t/t1013-read-tree-submodule.sh                |   1 +
 t/t1306-xdg-files.sh                          |   1 +
 t/t1350-config-hooks-path.sh                  |   1 +
 t/t1400-update-ref.sh                         |   2 +
 t/t2013-checkout-submodule.sh                 |   1 +
 t/t2024-checkout-dwim.sh                      |   1 +
 t/t2060-switch.sh                             |   1 +
 t/t2405-worktree-submodule.sh                 |   1 +
 t/t3007-ls-files-recurse-submodules.sh        |   1 +
 t/t3203-branch-output.sh                      |   2 +
 t/t3415-rebase-autosquash.sh                  |   1 +
 t/t3426-rebase-submodule.sh                   |   1 +
 t/t3512-cherry-pick-submodule.sh              |   1 +
 t/t3513-revert-submodule.sh                   |   1 +
 t/t3600-rm.sh                                 |   1 +
 t/t3906-stash-submodule.sh                    |   1 +
 t/t4001-diff-rename.sh                        |   4 +-
 t/t4041-diff-submodule-option.sh              |   1 +
 t/t4043-diff-rename-binary.sh                 |   1 +
 t/t4059-diff-submodule-not-initialized.sh     |   1 +
 t/t4060-diff-submodule-option-diff-format.sh  |   1 +
 t/t4120-apply-popt.sh                         |   1 +
 t/t4137-apply-submodule.sh                    |   1 +
 t/t4210-log-i18n.sh                           |   2 +
 t/t5563-simple-http-auth.sh                   |   1 +
 t/t5564-http-proxy.sh                         |   1 +
 t/t5581-http-curl-verbose.sh                  |   1 +
 t/t6006-rev-list-format.sh                    |   1 +
 t/t6041-bisect-submodule.sh                   |   1 +
 t/t6400-merge-df.sh                           |   1 +
 t/t6412-merge-large-rename.sh                 |   1 +
 t/t6426-merge-skip-unneeded-updates.sh        |   1 +
 t/t6429-merge-sequence-rename-caching.sh      |   1 +
 t/t6438-submodule-directory-file-conflicts.sh |   1 +
 t/t7001-mv.sh                                 |   2 +
 t/t7005-editor.sh                             |   1 +
 t/t7102-reset.sh                              |   1 +
 t/t7112-reset-submodule.sh                    |   1 +
 t/t7417-submodule-path-url.sh                 |   1 +
 t/t7421-submodule-summary-add.sh              |   1 +
 t/t7423-submodule-symlinks.sh                 |   1 +
 t/t9129-git-svn-i18n-commitencoding.sh        |   1 -
 t/t9139-git-svn-non-utf8-commitencoding.sh    |   1 -
 t/t9200-git-cvsexportcommit.sh                |   1 +
 t/t9401-git-cvsserver-crlf.sh                 |   1 +
 t/t9600-cvsimport.sh                          |   1 +
 t/t9601-cvsimport-vendor-branch.sh            |   1 +
 t/t9602-cvsimport-branches-tags.sh            |   1 +
 t/t9603-cvsimport-patchsets.sh                |   2 +
 t/t9604-cvsimport-timestamps.sh               |   2 +
 t/unit-tests/t-strvec.c                       | 269 +++++++
 t/unit-tests/test-lib.c                       |  13 +
 t/unit-tests/test-lib.h                       |  13 +
 transport-helper.c                            |   6 +-
 transport.c                                   |   1 +
 upload-pack.c                                 |   2 +-
 userdiff.h                                    |  12 +-
 109 files changed, 1151 insertions(+), 586 deletions(-)
 create mode 100644 t/unit-tests/t-strvec.c

Range-diff against v2:
1: 857b8b14ce ! 1: ded220a06b ci: add missing dependency for TTY prereq
@@ Commit message
systems are missing this dependency and thus don't execute those tests at all. Fix this.

+ Signed-off-by: Patrick Steinhardt <[email protected]>
+
## ci/install-dependencies.sh ##
@@ ci/install-dependencies.sh: alpine-*)
apk add --update shadow sudo build-base curl-dev openssl-dev expat-dev gettext \
2: ceade7dbba ! 2: f305e4bc46 t: mark a bunch of tests as leak-free
@@ Commit message
- t7423: Introduced via b20c10fd9b (t7423: add tests for symlinked submodule directories, 2024-01-28), passes since e8d0608944 (submodule: require the submodule path to contain directories only,
- 2024-03-26). The fix is not ovbiously related, but probably works
+ 2024-03-26). The fix is not obviously related, but probably works
because we now die early in many code paths.

- t9xxx: All of these are exercising CVS-related tooling and pass since at least Git v2.40. It's likely that these pass for a long
- time already, but nobody ever noticed because noone has CVS on their
- machine.
+ time already, but nobody ever noticed because Git developers do not
+ tend to have CVS on their machines.

Mark all of these tests as passing.

 3:  a96b5ac359 =  3:  9e946c1a83 transport-helper: fix leaking helper name
 4:  9dd8709d1b !  4:  aa5cbd9d14 strbuf: fix leak when `appendwholeline()` fails with EOF

@@ Commit message

In strbuf_appendwholeline() we call strbuf_getwholeline() with a temporary buffer. In case the call returns an error we indicate this by
- returning EOF, but never release the temporary buffer. This can lead to
- a memory leak when the line has been partially filled. Fix this.
+ returning EOF, but never release the temporary buffer. This can cause a
+ leak though because strbuf_getwholeline() calls getline(3). Quoting
+ its documentation:
+
+ If *lineptr was set to NULL before the call, then the buffer
+ should be freed by the user program even on failure.
+
+ Consequently, the temporary buffer may hold allocated memory even when
+ the call to strbuf_getwholeline() fails.
+
+ Fix this by releasing the temporary buffer on error.

 5:  6d4e9ce706 =  5:  4a6dd9b6a8 checkout: clarify memory ownership in `unique_tracking_name()`
 6:  141cae2de1 =  6:  fa91a3942e http: refactor code to clarify memory ownership
 7:  ff5e761e55 =  7:  88babf1abf config: clarify memory ownership in `git_config_pathname()`
 8:  afe69c7303 =  8:  feec7e971f diff: refactor code to clarify memory ownership of prefixes
 9:  eb7fce55b0 =  9:  dae00f1b63 convert: refactor code to clarify ownership of check_roundtrip_encoding
10:  ee2fcf388c = 10:  02c5be27be builtin/log: stop using globals for log config
11:  3490ad3a02 = 11:  eeba79678a builtin/log: stop using globals for format config
12:  6cfc28c7e2 ! 12:  a4fafcd079 config: clarify memory ownership in `git_config_string()`
    @@ config.c: int git_config_bool(const char *name, const char *value)
      {
      	if (!value)
      		return config_error_nonbool(var);
    +@@ config.c: static int git_default_core_config(const char *var, const char *value,
    + 
    + 	if (!strcmp(var, "core.checkroundtripencoding")) {
    + 		FREE_AND_NULL(check_roundtrip_encoding);
    +-		return git_config_string((const char **) &check_roundtrip_encoding, var, value);
    ++		return git_config_string(&check_roundtrip_encoding, var, value);
    + 	}
    + 
    + 	if (!strcmp(var, "core.notesref")) {
     @@ config.c: int git_configset_get_string(struct config_set *set, const char *key, char **des
      {
      	const char *value;
13:  70e8e26513 ! 13:  8b74dff678 config: plug various memory leaks
    @@ config.c: static int git_default_core_config(const char *var, const char *value,
      		return git_config_pathname(&git_hooks_path, var, value);
      	}
      
    -@@ config.c: static int git_default_core_config(const char *var, const char *value,
    - 
    - 	if (!strcmp(var, "core.checkroundtripencoding")) {
    - 		FREE_AND_NULL(check_roundtrip_encoding);
    --		return git_config_string((const char **) &check_roundtrip_encoding, var, value);
    -+		return git_config_string(&check_roundtrip_encoding, var, value);
    - 	}
    - 
    - 	if (!strcmp(var, "core.notesref")) {
     @@ config.c: static int git_default_core_config(const char *var, const char *value,
      		return 0;
      	}
14:  f1a1c43e76 = 14:  265665fe6c builtin/credential: clear credential before exit
15:  64b92156f8 = 15:  b315a5bb5c commit-reach: fix memory leak in `ahead_behind()`
16:  cd8a992f08 = 16:  7c75a94756 submodule: fix leaking memory for submodule entries
17:  128e2eaf7a = 17:  0f61ee9929 strvec: add functions to replace and remove strings
18:  1310b24fc2 = 18:  1830e2a568 builtin/mv: refactor `add_slash()` to always return allocated strings
19:  d4fef9825a = 19:  9eeafac365 builtin/mv duplicate string list memory
20:  797cdb286a = 20:  48b9d3e343 builtin/mv: refactor to use `struct strvec`
21:  095469193c = 21:  add7946446 builtin/mv: fix leaks for submodule gitfile paths

2.45.1.246.gb9cfe4845c.dirty