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

[PATCH v2 1/3] config: free resources of `struct config_store_data`

From
MÅMartin Ågren <martin.agren@gmail.com>
Date
May 20, 2018, 10:42 UTC
Message-ID
<ecae89b852bb98bab1cc90517bfcabfbe6eeafe8.1526812503.git.martin.agren@gmail.com>
In-Reply-To
<cover.1526812503.git.martin.agren@gmail.com>

Commit fee8572c6d (config: avoid using the global variable `store`, 2018-04-09) dropped the staticness of a certain struct, instead letting the users create an instance on the stack and pass around a pointer.

We do not free all the memory that the struct tracks. When the struct was static, the memory would always be reachable. Now that we keep the struct on the stack, though, as soon as we return, it goes out of scope and we leak the memory it points to. In particular, we leak the memory pointed to by the `parsed` and `seen` fields.

Introduce and use a helper function `config_store_data_clear()` to plug these leaks. The memory tracked here is config parser events. Once the users (`git_config_set_multivar_in_file_gently()` and `git_config_copy_or_rename_section_in_file()` at the moment) are done, no-one should be holding on to a pointer into this memory.

There are two more members of the struct that are candidates for freeing in this new function (`key` and `value_regex`). Those are actually already being taken care of. The next couple of patches will move their freeing into the function we are adding here.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 config.c | 9 +++++++++
 1 file changed, 9 insertions(+)
diff --git a/config.c b/config.c
index 6f8f1d8c11..b3282f7193 100644
--- a/config.c
+++ b/config.c
@@ -2333,6 +2333,13 @@ struct config_store_data {
 	unsigned int key_seen:1, section_seen:1, is_keys_section:1;
 };
 
+static void config_store_data_clear(struct config_store_data *store)
+{
+	free(store->parsed);
+	free(store->seen);
+	memset(store, 0, sizeof(*store));
+}
+
 static int matches(const char *key, const char *value,
 		   const struct config_store_data *store)
 {
@@ -2887,6 +2894,7 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
 		munmap(contents, contents_sz);
 	if (in_fd >= 0)
 		close(in_fd);
+	config_store_data_clear(&store);
 	return ret;
 
 write_err_out:
@@ -3127,6 +3135,7 @@ static int git_config_copy_or_rename_section_in_file(const char *config_filename
 	rollback_lock_file(&lock);
 out_no_rollback:
 	free(filename_buf);
+	config_store_data_clear(&store);
 	return ret;
 }
 
-- 
2.17.0.840.g5d83f92caf
Previous: Martin ÅgrenNext: Martin Ågren
Message 13 of 18 in “config: free resources of `struct config_store_data`”
  1. config: free resources of `struct config_store_data`Martin Ågren, May 13, 2018
  2. Eric SunshineMay 13, 2018
  3. Martin ÅgrenMay 13, 2018
  4. 2/1 config: let `config_store_data_clear()` handle `value_regex`Martin Ågren, May 13, 2018
  5. Eric SunshineMay 14, 2018
  6. regex: do not call `regfree()` if compilation failsMartin Ågren, May 20, 2018
  7. Stefan BellerMay 21, 2018
  8. Eric SunshineMay 22, 2018
  9. Martin ÅgrenMay 22, 2018
  10. 3/1 config: let `config_store_data_clear()` handle `key`Martin Ågren, May 13, 2018
  11. Eric SunshineMay 14, 2018
  12. 0/3 config: free resources of `struct config_store_data`Martin Ågren, May 20, 2018
  13. 1/3 config: free resources of `struct config_store_data`Martin Ågren, May 20, 2018
  14. 2/3 config: let `config_store_data_clear()` handle `value_regex`Martin Ågren, May 20, 2018
  15. 3/3 config: let `config_store_data_clear()` handle `key`Martin Ågren, May 20, 2018
  16. Eric SunshineMay 23, 2018
  17. Eric SunshineMay 23, 2018
  18. Martin ÅgrenMay 13, 2018

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.