[PATCH 11/11] blame: use "dup" string_list for ignore-revs files

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



We feed items into ignore_revs_file_list in two ways: from config and
from the command-line. Items from the command-line point to argv entries
that we don't own, but items from config are hea-allocated strings whose
ownership is handed to the string list when we insert them.

Because the string_list is declared with "NODUP", our string_list_clear()
call will not free the entries themselves. This is the right thing for
the argv entries, but means that we leak the allocated config entries.

Let's declare the string list as owning its own strings, which means
that the argv entries will be copied.

For the config entries we would ideally use string_list_append_nodup()
to just hand off ownership of our strings. But we insert them into the
sorted list with string_list_insert(), which doesn't have a nodup
variant. Since this isn't a hot path, we can accept that the path
interpolation will produce a temporary string which is then freed.

Signed-off-by: Jeff King <peff@xxxxxxxx>
---
Not strictly related to this series, but something I noticed while
converting this spot in an earlier patch. I had thought originally we
could switch to avoid the extra copy altogether:

  if (!value)
	return config_error_nonbool();
  string_list_insert(&ignore_revs_file_list, value);

but of course that is missing the call to interpolate_path().

I imagine that it could also be further refactored to append while
reading the config, and then sort after. That's more efficient overall,
and would let us use append_nodup().

 builtin/blame.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 0b07ceb110..fa7f8fce09 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -67,7 +67,7 @@ static int no_whole_file_rename;
 static int show_progress;
 static char repeated_meta_color[COLOR_MAXLEN];
 static int coloring_mode;
-static struct string_list ignore_revs_file_list = STRING_LIST_INIT_NODUP;
+static struct string_list ignore_revs_file_list = STRING_LIST_INIT_DUP;
 static int mark_unblamable_lines;
 static int mark_ignored_lines;
 
@@ -725,6 +725,7 @@ static int git_blame_config(const char *var, const char *value,
 		if (ret)
 			return ret;
 		string_list_insert(&ignore_revs_file_list, str);
+		free(str);
 		return 0;
 	}
 	if (!strcmp(var, "blame.markunblamablelines")) {
-- 
2.44.0.872.g288abe5b5b




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux