All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Martin Ågren" <martin.agren@gmail.com>
To: Eric Sunshine <sunshine@sunshineco.com>
Cc: git@vger.kernel.org, Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: [PATCH v2 1/3] config: free resources of `struct config_store_data`
Date: Sun, 20 May 2018 12:42:33 +0200	[thread overview]
Message-ID: <ecae89b852bb98bab1cc90517bfcabfbe6eeafe8.1526812503.git.martin.agren@gmail.com> (raw)
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


  reply	other threads:[~2018-05-20 10:43 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-13  8:23 [PATCH] config: free resources of `struct config_store_data` Martin Ågren
2018-05-13  8:59 ` Eric Sunshine
2018-05-13  9:58   ` Martin Ågren
2018-05-13  9:58     ` [PATCH 2/1] config: let `config_store_data_clear()` handle `value_regex` Martin Ågren
2018-05-14  3:05       ` Eric Sunshine
2018-05-20 10:50         ` [PATCH] regex: do not call `regfree()` if compilation fails Martin Ågren
2018-05-21 18:43           ` Stefan Beller
2018-05-22  2:20             ` Eric Sunshine
2018-05-22 11:00               ` Martin Ågren
2018-05-13  9:58     ` [PATCH 3/1] config: let `config_store_data_clear()` handle `key` Martin Ågren
2018-05-14  3:03     ` [PATCH] config: free resources of `struct config_store_data` Eric Sunshine
2018-05-20 10:42       ` [PATCH v2 0/3] " Martin Ågren
2018-05-20 10:42         ` Martin Ågren [this message]
2018-05-20 10:42         ` [PATCH v2 2/3] config: let `config_store_data_clear()` handle `value_regex` Martin Ågren
2018-05-20 10:42         ` [PATCH v2 3/3] config: let `config_store_data_clear()` handle `key` Martin Ågren
2018-05-23  7:03           ` Eric Sunshine
2018-05-23  7:01         ` [PATCH v2 0/3] config: free resources of `struct config_store_data` Eric Sunshine
2018-05-13 18:40 ` [PATCH] " Martin Ågren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ecae89b852bb98bab1cc90517bfcabfbe6eeafe8.1526812503.git.martin.agren@gmail.com \
    --to=martin.agren@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=johannes.schindelin@gmx.de \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.