git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 06/21] http: refactor code to clarify memory ownership

From
Patrick Steinhardt <ps@pks.im>
Date
May 27, 2024, 11:46 UTC
Message-ID
<fa91a3942edd2c26389a25bb2a5c1737151e7c12.1716810168.git.ps@pks.im>
In-Reply-To
<cover.1716810168.git.ps@pks.im>

There are various variables assigned via `git_config_string()` and `git_config_pathname()` which are never free'd. This bug is relatable because the out parameter of those functions are a `const char **`, even though memory ownership is transferred to the caller.

We're about to adapt the functions to instead use `char **`. Prepare the code accordingly. Note that the `(const char **)` casts will go away once we have adapted the functions.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 http.c | 62 ++++++++++++++++++++++++++++++----------------------------
 1 file changed, 32 insertions(+), 30 deletions(-)
diff --git a/http.c b/http.c
index 752c879c1f..db2e2f1d39 100644
--- a/http.c
+++ b/http.c
@@ -39,8 +39,8 @@ char curl_errorstr[CURL_ERROR_SIZE];
 static int curl_ssl_verify = -1;
 static int curl_ssl_try;
 static const char *curl_http_version = NULL;
-static const char *ssl_cert;
-static const char *ssl_cert_type;
+static char *ssl_cert;
+static char *ssl_cert_type;
 static const char *ssl_cipherlist;
 static const char *ssl_version;
 static struct {
@@ -59,23 +59,23 @@ static struct {
 	{ "tlsv1.3", CURL_SSLVERSION_TLSv1_3 },
 #endif
 };
-static const char *ssl_key;
-static const char *ssl_key_type;
-static const char *ssl_capath;
-static const char *curl_no_proxy;
+static char *ssl_key;
+static char *ssl_key_type;
+static char *ssl_capath;
+static char *curl_no_proxy;
 #ifdef GIT_CURL_HAVE_CURLOPT_PINNEDPUBLICKEY
 static const char *ssl_pinnedkey;
 #endif
-static const char *ssl_cainfo;
+static char *ssl_cainfo;
 static long curl_low_speed_limit = -1;
 static long curl_low_speed_time = -1;
 static int curl_ftp_no_epsv;
-static const char *curl_http_proxy;
-static const char *http_proxy_authmethod;
+static char *curl_http_proxy;
+static char *http_proxy_authmethod;
 
-static const char *http_proxy_ssl_cert;
-static const char *http_proxy_ssl_key;
-static const char *http_proxy_ssl_ca_info;
+static char *http_proxy_ssl_cert;
+static char *http_proxy_ssl_key;
+static char *http_proxy_ssl_ca_info;
 static struct credential proxy_cert_auth = CREDENTIAL_INIT;
 static int proxy_ssl_cert_password_required;
 
@@ -112,7 +112,7 @@ static const char *curl_cookie_file;
 static int curl_save_cookies;
 struct credential http_auth = CREDENTIAL_INIT;
 static int http_proactive_auth;
-static const char *user_agent;
+static char *user_agent;
 static int curl_empty_auth = -1;
 
 enum http_follow_config http_follow_config = HTTP_FOLLOW_INITIAL;
@@ -381,17 +381,17 @@ static int http_options(const char *var, const char *value,
 	if (!strcmp("http.sslversion", var))
 		return git_config_string(&ssl_version, var, value);
 	if (!strcmp("http.sslcert", var))
-		return git_config_pathname(&ssl_cert, var, value);
+		return git_config_pathname((const char **)&ssl_cert, var, value);
 	if (!strcmp("http.sslcerttype", var))
-		return git_config_string(&ssl_cert_type, var, value);
+		return git_config_string((const char **)&ssl_cert_type, var, value);
 	if (!strcmp("http.sslkey", var))
-		return git_config_pathname(&ssl_key, var, value);
+		return git_config_pathname((const char **)&ssl_key, var, value);
 	if (!strcmp("http.sslkeytype", var))
-		return git_config_string(&ssl_key_type, var, value);
+		return git_config_string((const char **)&ssl_key_type, var, value);
 	if (!strcmp("http.sslcapath", var))
-		return git_config_pathname(&ssl_capath, var, value);
+		return git_config_pathname((const char **)&ssl_capath, var, value);
 	if (!strcmp("http.sslcainfo", var))
-		return git_config_pathname(&ssl_cainfo, var, value);
+		return git_config_pathname((const char **)&ssl_cainfo, var, value);
 	if (!strcmp("http.sslcertpasswordprotected", var)) {
 		ssl_cert_password_required = git_config_bool(var, value);
 		return 0;
@@ -440,19 +440,19 @@ static int http_options(const char *var, const char *value,
 		return 0;
 	}
 	if (!strcmp("http.proxy", var))
-		return git_config_string(&curl_http_proxy, var, value);
+		return git_config_string((const char **)&curl_http_proxy, var, value);
 
 	if (!strcmp("http.proxyauthmethod", var))
-		return git_config_string(&http_proxy_authmethod, var, value);
+		return git_config_string((const char **)&http_proxy_authmethod, var, value);
 
 	if (!strcmp("http.proxysslcert", var))
-		return git_config_string(&http_proxy_ssl_cert, var, value);
+		return git_config_string((const char **)&http_proxy_ssl_cert, var, value);
 
 	if (!strcmp("http.proxysslkey", var))
-		return git_config_string(&http_proxy_ssl_key, var, value);
+		return git_config_string((const char **)&http_proxy_ssl_key, var, value);
 
 	if (!strcmp("http.proxysslcainfo", var))
-		return git_config_string(&http_proxy_ssl_ca_info, var, value);
+		return git_config_string((const char **)&http_proxy_ssl_ca_info, var, value);
 
 	if (!strcmp("http.proxysslcertpasswordprotected", var)) {
 		proxy_ssl_cert_password_required = git_config_bool(var, value);
@@ -476,7 +476,7 @@ static int http_options(const char *var, const char *value,
 	}
 
 	if (!strcmp("http.useragent", var))
-		return git_config_string(&user_agent, var, value);
+		return git_config_string((const char **)&user_agent, var, value);
 
 	if (!strcmp("http.emptyauth", var)) {
 		if (value && !strcmp("auto", value))
@@ -592,10 +592,10 @@ static void init_curl_http_auth(CURL *result)
 }
 
 /* *var must be free-able */
-static void var_override(const char **var, char *value)
+static void var_override(char **var, char *value)
 {
 	if (value) {
-		free((void *)*var);
+		free(*var);
 		*var = xstrdup(value);
 	}
 }
@@ -1233,11 +1233,13 @@ static CURL *get_curl_handle(void)
 	return result;
 }
 
-static void set_from_env(const char **var, const char *envname)
+static void set_from_env(char **var, const char *envname)
 {
 	const char *val = getenv(envname);
-	if (val)
-		*var = val;
+	if (val) {
+		FREE_AND_NULL(*var);
+		*var = xstrdup(val);
+	}
 }
 
 void http_init(struct remote *remote, const char *url, int proactive_auth)
-- 
2.45.1.246.gb9cfe4845c.dirty
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 83 of 115 in “Various memory leak fixes”
  1. 00/20 Various memory leak fixesPatrick Steinhardt, May 23, 2024
  2. 01/20 t: mark a bunch of tests as leak-freePatrick Steinhardt, May 23, 2024
  3. Junio C HamanoMay 23, 2024
  4. Patrick SteinhardtMay 24, 2024
  5. Junio C HamanoMay 24, 2024
  6. Junio C HamanoMay 24, 2024
  7. Karthik NayakMay 24, 2024
  8. 02/20 transport-helper: fix leaking helper namePatrick Steinhardt, May 23, 2024
  9. Junio C HamanoMay 23, 2024
  10. Karthik NayakMay 24, 2024
  11. 03/20 strbuf: fix leak when `appendwholeline()` fails with EOFPatrick Steinhardt, May 23, 2024
  12. 04/20 checkout: clarify memory ownership in `unique_tracking_name()`Patrick Steinhardt, May 23, 2024
  13. 05/20 http: refactor code to clarify memory ownershipPatrick Steinhardt, May 23, 2024
  14. 06/20 config: clarify memory ownership in `git_config_pathname()`Patrick Steinhardt, May 23, 2024
  15. 07/20 diff: refactor code to clarify memory ownership of prefixesPatrick Steinhardt, May 23, 2024
  16. Eric SunshineMay 23, 2024
  17. 08/20 convert: refactor code to clarify ownership of check_roundtrip_encodingPatrick Steinhardt, May 23, 2024
  18. 09/20 builtin/log: stop using globals for log configPatrick Steinhardt, May 23, 2024
  19. 10/20 builtin/log: stop using globals for format configPatrick Steinhardt, May 23, 2024
  20. 11/20 config: clarify memory ownership in `git_config_string()`Patrick Steinhardt, May 23, 2024
  21. 12/20 config: plug various memory leaksPatrick Steinhardt, May 23, 2024
  22. Junio C HamanoMay 23, 2024
  23. Patrick SteinhardtMay 24, 2024
  24. Patrick SteinhardtMay 24, 2024
  25. Junio C HamanoMay 24, 2024
  26. Junio C HamanoMay 24, 2024
  27. 13/20 builtin/credential: clear credential before exitPatrick Steinhardt, May 23, 2024
  28. 14/20 commit-reach: fix memory leak in `ahead_behind()`Patrick Steinhardt, May 23, 2024
  29. 15/20 submodule: fix leaking memory for submodule entriesPatrick Steinhardt, May 23, 2024
  30. 16/20 strvec: add functions to replace and remove stringsPatrick Steinhardt, May 23, 2024
  31. Eric SunshineMay 23, 2024
  32. Patrick SteinhardtMay 24, 2024
  33. 17/20 builtin/mv: refactor `add_slash()` to always return allocated stringsPatrick Steinhardt, May 23, 2024
  34. 18/20 builtin/mv duplicate string list memoryPatrick Steinhardt, May 23, 2024
  35. 19/20 builtin/mv: refactor to use `struct strvec`Patrick Steinhardt, May 23, 2024
  36. 20/20 builtin/mv: fix leaks for submodule gitfile pathsPatrick Steinhardt, May 23, 2024
  37. Junio C HamanoMay 23, 2024
  38. Patrick SteinhardtMay 24, 2024
  39. 00/21 Various memory leak fixesPatrick Steinhardt, May 24, 2024
  40. 01/21 ci: add missing dependency for TTY prereqPatrick Steinhardt, May 24, 2024
  41. Junio C HamanoMay 24, 2024
  42. 02/21 t: mark a bunch of tests as leak-freePatrick Steinhardt, May 24, 2024
  43. 03/21 transport-helper: fix leaking helper namePatrick Steinhardt, May 24, 2024
  44. 04/21 strbuf: fix leak when `appendwholeline()` fails with EOFPatrick Steinhardt, May 24, 2024
  45. Jeff KingMay 25, 2024
  46. Patrick SteinhardtMay 27, 2024
  47. Jeff KingMay 29, 2024
  48. Patrick SteinhardtMay 29, 2024
  49. Jeff KingMay 30, 2024
  50. 05/21 checkout: clarify memory ownership in `unique_tracking_name()`Patrick Steinhardt, May 24, 2024
  51. 06/21 http: refactor code to clarify memory ownershipPatrick Steinhardt, May 24, 2024
  52. 07/21 config: clarify memory ownership in `git_config_pathname()`Patrick Steinhardt, May 24, 2024
  53. 08/21 diff: refactor code to clarify memory ownership of prefixesPatrick Steinhardt, May 24, 2024
  54. 09/21 convert: refactor code to clarify ownership of check_roundtrip_encodingPatrick Steinhardt, May 24, 2024
  55. 10/21 builtin/log: stop using globals for log configPatrick Steinhardt, May 24, 2024
  56. 11/21 builtin/log: stop using globals for format configPatrick Steinhardt, May 24, 2024
  57. 12/21 config: clarify memory ownership in `git_config_string()`Patrick Steinhardt, May 24, 2024
  58. 13/21 config: plug various memory leaksPatrick Steinhardt, May 24, 2024
  59. Patrick SteinhardtMay 24, 2024
  60. Jeff KingMay 25, 2024
  61. Patrick SteinhardtMay 27, 2024
  62. Jeff KingMay 29, 2024
  63. 14/21 builtin/credential: clear credential before exitPatrick Steinhardt, May 24, 2024
  64. 15/21 commit-reach: fix memory leak in `ahead_behind()`Patrick Steinhardt, May 24, 2024
  65. 16/21 submodule: fix leaking memory for submodule entriesPatrick Steinhardt, May 24, 2024
  66. 17/21 strvec: add functions to replace and remove stringsPatrick Steinhardt, May 24, 2024
  67. 18/21 builtin/mv: refactor `add_slash()` to always return allocated stringsPatrick Steinhardt, May 24, 2024
  68. 19/21 builtin/mv duplicate string list memoryPatrick Steinhardt, May 24, 2024
  69. 20/21 builtin/mv: refactor to use `struct strvec`Patrick Steinhardt, May 24, 2024
  70. 21/21 builtin/mv: fix leaks for submodule gitfile pathsPatrick Steinhardt, May 24, 2024
  71. Junio C HamanoMay 25, 2024
  72. Patrick SteinhardtMay 27, 2024
  73. Junio C HamanoMay 27, 2024
  74. Junio C HamanoMay 27, 2024
  75. Patrick SteinhardtMay 28, 2024
  76. Karthik NayakMay 29, 2024
  77. 00/21 Various memory leak fixesPatrick Steinhardt, May 27, 2024
  78. 01/21 ci: add missing dependency for TTY prereqPatrick Steinhardt, May 27, 2024
  79. 02/21 t: mark a bunch of tests as leak-freePatrick Steinhardt, May 27, 2024
  80. 03/21 transport-helper: fix leaking helper namePatrick Steinhardt, May 27, 2024
  81. 04/21 strbuf: fix leak when `appendwholeline()` fails with EOFPatrick Steinhardt, May 27, 2024
  82. 05/21 checkout: clarify memory ownership in `unique_tracking_name()`Patrick Steinhardt, May 27, 2024
  83. 06/21 http: refactor code to clarify memory ownershipPatrick Steinhardt, May 27, 2024
  84. 07/21 config: clarify memory ownership in `git_config_pathname()`Patrick Steinhardt, May 27, 2024
  85. 08/21 diff: refactor code to clarify memory ownership of prefixesPatrick Steinhardt, May 27, 2024
  86. 09/21 convert: refactor code to clarify ownership of check_roundtrip_encodingPatrick Steinhardt, May 27, 2024
  87. 10/21 builtin/log: stop using globals for log configPatrick Steinhardt, May 27, 2024
  88. 11/21 builtin/log: stop using globals for format configPatrick Steinhardt, May 27, 2024
  89. 12/21 config: clarify memory ownership in `git_config_string()`Patrick Steinhardt, May 27, 2024
  90. 13/21 config: plug various memory leaksPatrick Steinhardt, May 27, 2024
  91. 14/21 builtin/credential: clear credential before exitPatrick Steinhardt, May 27, 2024
  92. 15/21 commit-reach: fix memory leak in `ahead_behind()`Patrick Steinhardt, May 27, 2024
  93. 16/21 submodule: fix leaking memory for submodule entriesPatrick Steinhardt, May 27, 2024
  94. 17/21 strvec: add functions to replace and remove stringsPatrick Steinhardt, May 27, 2024
  95. 18/21 builtin/mv: refactor `add_slash()` to always return allocated stringsPatrick Steinhardt, May 27, 2024
  96. 19/21 builtin/mv duplicate string list memoryPatrick Steinhardt, May 27, 2024
  97. 20/21 builtin/mv: refactor to use `struct strvec`Patrick Steinhardt, May 27, 2024
  98. 21/21 builtin/mv: fix leaks for submodule gitfile pathsPatrick Steinhardt, May 27, 2024
  99. Junio C HamanoMay 27, 2024
  100. 0/5 add-ons for ps/leakfixesJeff King, May 30, 2024
  101. 1/5 t-strvec: use va_end() to match va_start()Jeff King, May 30, 2024
  102. 2/5 t-strvec: mark variable-arg helper with LAST_ARG_MUST_BE_NULLJeff King, May 30, 2024
  103. 3/5 mv: move src_dir cleanup to end of cmd_mv()Jeff King, May 30, 2024
  104. Patrick SteinhardtMay 30, 2024
  105. Jeff KingMay 30, 2024
  106. Patrick SteinhardtMay 30, 2024
  107. Jeff KingMay 30, 2024
  108. Patrick SteinhardtMay 30, 2024
  109. Jeff KingMay 30, 2024
  110. 4/5 mv: factor out empty src_dir removalJeff King, May 30, 2024
  111. 5/5 mv: replace src_dir with a strvecJeff King, May 30, 2024
  112. Junio C HamanoMay 30, 2024
  113. Jeff KingMay 31, 2024
  114. Junio C HamanoMay 31, 2024
  115. Patrick SteinhardtMay 30, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.