git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Anders Waldenborg <anders@0x63.nu>
To: git@vger.kernel.org
Cc: Anders Waldenborg <anders@0x63.nu>,
	christian.couder@gmail.com, peff@peff.net,
	jonathantanmy@google.com
Subject: [PATCH 07/21] trailer: simplify 'arg_item' lifetime
Date: Sun, 25 Oct 2020 22:26:38 +0100	[thread overview]
Message-ID: <20201025212652.3003036-8-anders@0x63.nu> (raw)
In-Reply-To: <20201025212652.3003036-1-anders@0x63.nu>

); SAEximRunCond expanded to false

'struct arg_item' are created from config and '--trailers' arguments
in 'git interpret-trailers'.

Then they were freed as they were processed. This made it harder to
reason about and ensure that all of them were properly freed in all
cases.

This commit extends the lifetime by not doing any freeing during
processing but rather freeing the whole list afterwards. This make it
clearer and will allow keeping a reference to the config stored in the
arg item.

The drawback is that there is extra memory allocation as previously
the strings could be donated to the trailer_item when that is
created. Now they have to be copied.

No functional change intended.

Signed-off-by: Anders Waldenborg <anders@0x63.nu>
---
 trailer.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/trailer.c b/trailer.c
index 227df1c0ef..047781463a 100644
--- a/trailer.c
+++ b/trailer.c
@@ -177,13 +177,11 @@ static void print_all(FILE *outfile, struct list_head *head,
 	}
 }
 
-static struct trailer_item *trailer_from_arg(struct arg_item *arg_tok)
+static struct trailer_item *trailer_from_arg(const struct arg_item *arg_tok)
 {
 	struct trailer_item *new_item = xcalloc(sizeof(*new_item), 1);
-	new_item->token = arg_tok->token;
-	new_item->value = arg_tok->value;
-	arg_tok->token = arg_tok->value = NULL;
-	free_arg_item(arg_tok);
+	new_item->token = xstrdup(arg_tok->token);
+	new_item->value = xstrdup(arg_tok->value);
 	return new_item;
 }
 
@@ -274,7 +272,6 @@ static void apply_arg_if_exists(struct trailer_item *in_tok,
 {
 	switch (arg_tok->conf.if_exists) {
 	case EXISTS_DO_NOTHING:
-		free_arg_item(arg_tok);
 		break;
 	case EXISTS_REPLACE:
 		apply_item_command(in_tok, arg_tok);
@@ -290,15 +287,11 @@ static void apply_arg_if_exists(struct trailer_item *in_tok,
 		apply_item_command(in_tok, arg_tok);
 		if (check_if_different(in_tok, arg_tok, 1, head))
 			add_arg_to_input_list(on_tok, arg_tok);
-		else
-			free_arg_item(arg_tok);
 		break;
 	case EXISTS_ADD_IF_DIFFERENT_NEIGHBOR:
 		apply_item_command(in_tok, arg_tok);
 		if (check_if_different(on_tok, arg_tok, 0, head))
 			add_arg_to_input_list(on_tok, arg_tok);
-		else
-			free_arg_item(arg_tok);
 		break;
 	default:
 		BUG("trailer.c: unhandled value %d",
@@ -314,7 +307,6 @@ static void apply_arg_if_missing(struct list_head *head,
 
 	switch (arg_tok->conf.if_missing) {
 	case MISSING_DO_NOTHING:
-		free_arg_item(arg_tok);
 		break;
 	case MISSING_ADD:
 		where = arg_tok->conf.where;
@@ -364,15 +356,13 @@ static int find_same_and_apply_arg(struct list_head *head,
 static void process_trailers_lists(struct list_head *head,
 				   struct list_head *arg_head)
 {
-	struct list_head *pos, *p;
+	struct list_head *pos;
 	struct arg_item *arg_tok;
 
-	list_for_each_safe(pos, p, arg_head) {
+	list_for_each(pos, arg_head) {
 		int applied = 0;
 		arg_tok = list_entry(pos, struct arg_item, list);
 
-		list_del(pos);
-
 		applied = find_same_and_apply_arg(head, arg_tok);
 
 		if (!applied)
@@ -999,6 +989,15 @@ static void free_all_trailer_items(struct list_head *head)
 	}
 }
 
+static void free_all_arg_items(struct list_head *head)
+{
+	struct list_head *pos, *p;
+	list_for_each_safe(pos, p, head) {
+		list_del(pos);
+		free_arg_item(list_entry(pos, struct arg_item, list));
+	}
+}
+
 static struct tempfile *trailers_tempfile;
 
 static FILE *create_in_place_tempfile(const char *file)
@@ -1035,6 +1034,7 @@ void process_trailers(const char *file,
 		      struct list_head *new_trailer_head)
 {
 	LIST_HEAD(head);
+	LIST_HEAD(arg_head);
 	struct strbuf sb = STRBUF_INIT;
 	size_t trailer_end;
 	FILE *outfile = stdout;
@@ -1050,7 +1050,6 @@ void process_trailers(const char *file,
 	trailer_end = process_input_file(outfile, sb.buf, &head, opts);
 
 	if (!opts->only_input) {
-		LIST_HEAD(arg_head);
 		process_command_line_args(&arg_head, new_trailer_head);
 		process_trailers_lists(&head, &arg_head);
 	}
@@ -1058,6 +1057,7 @@ void process_trailers(const char *file,
 	print_all(outfile, &head, opts);
 
 	free_all_trailer_items(&head);
+	free_all_arg_items(&arg_head);
 
 	/* Print the lines after the trailers as is */
 	if (!opts->only_trailers)
-- 
2.25.1


  parent reply	other threads:[~2020-10-25 22:42 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-25 21:26 [PATCH 00/21] trailer fixes Anders Waldenborg
2020-10-25 21:26 ` [PATCH 01/21] trailer: change token_{from,matches}_item into taking conf_info Anders Waldenborg
2020-10-26 11:56   ` Christian Couder
2020-10-25 21:26 ` [PATCH 02/21] trailer: don't use 'struct arg_item' for storing config Anders Waldenborg
2020-10-25 21:26 ` [PATCH 03/21] doc: mention canonicalization in git i-t manual Anders Waldenborg
2020-10-26 12:14   ` Christian Couder
2020-10-25 21:26 ` [PATCH 04/21] pretty: allow using aliases in %(trailer:key=xyz) Anders Waldenborg
2020-10-26 12:38   ` Christian Couder
2020-10-25 21:26 ` [PATCH 05/21] trailer: rename 'free_all' to 'free_all_trailer_items' Anders Waldenborg
2020-10-26 12:42   ` Christian Couder
2020-11-10 19:52     ` Jeff King
2020-10-25 21:26 ` [PATCH 06/21] t4205: add test for trailer in log with nonstandard separator Anders Waldenborg
2020-10-26 12:43   ` Christian Couder
2020-11-09 22:12     ` Anders Waldenborg
2020-11-10  7:55       ` Christian Couder
2020-11-10 19:54       ` Jeff King
2020-10-25 21:26 ` Anders Waldenborg [this message]
2020-10-25 21:26 ` [PATCH 08/21] trailer: keep track of conf in trailer_item Anders Waldenborg
2020-11-10 19:58   ` Jeff King
2020-10-25 21:26 ` [PATCH 09/21] trailer: refactor print_tok_val into taking item Anders Waldenborg
2020-10-25 21:26 ` [PATCH 10/21] trailer: move trailer token canonicalization print time Anders Waldenborg
2020-10-25 21:26 ` [PATCH 11/21] trailer: remember separator used in input Anders Waldenborg
2020-10-25 21:26 ` [PATCH 12/21] trailer: handle configured nondefault separators explicitly Anders Waldenborg
2020-11-10 20:06   ` Jeff King
2020-10-25 21:26 ` [PATCH 13/21] trailer: add option to make canonicalization optional Anders Waldenborg
2020-11-10 20:10   ` Jeff King
2020-10-25 21:26 ` [PATCH 14/21] trailer: move skipping of blank lines to own loop when finding trailer Anders Waldenborg
2020-10-25 21:26 ` [PATCH 15/21] trailer: factor out classify_trailer_line Anders Waldenborg
2020-10-25 21:26 ` [PATCH 16/21] t7513: add failing test for configured trailing line classification Anders Waldenborg
2020-10-25 21:26 ` [PATCH 17/21] trailer: don't treat line with prefix of known trailer as known Anders Waldenborg
2020-11-10 20:16   ` Jeff King
2020-10-25 21:26 ` [PATCH 18/21] trailer: factor out config lookup to separate function Anders Waldenborg
2020-10-25 21:26 ` [PATCH 19/21] trailer: move config lookup out of parse_trailer Anders Waldenborg
2020-10-25 21:26 ` [PATCH 20/21] trailer: add failing tests for matching trailers against input Anders Waldenborg
2020-10-25 21:26 ` [PATCH 21/21] trailer: only do prefix matching for configured trailers on commandline Anders Waldenborg
2020-11-10  7:44 ` [PATCH 00/21] trailer fixes Christian Couder
2020-12-05  1:39 ` [PATCH 0/5] pretty format %(trailers): improve machine readability Ævar Arnfjörð Bjarmason
2020-12-05 18:18   ` Anders Waldenborg
2020-12-07  8:53     ` Ævar Arnfjörð Bjarmason
2020-12-06  0:24   ` [PATCH v2 " Ævar Arnfjörð Bjarmason
2020-12-09 15:52     ` [PATCH v3 " Ævar Arnfjörð Bjarmason
2020-12-10 10:48       ` Christian Couder
2020-12-10 19:00         ` Junio C Hamano
2020-12-09 15:52     ` [PATCH v3 1/5] pretty format %(trailers) test: split a long line Ævar Arnfjörð Bjarmason
2020-12-09 15:52     ` [PATCH v3 2/5] pretty format %(trailers) doc: avoid repetition Ævar Arnfjörð Bjarmason
2020-12-10 19:01       ` Junio C Hamano
2020-12-09 15:52     ` [PATCH v3 3/5] pretty-format %(trailers): fix broken standalone "valueonly" Ævar Arnfjörð Bjarmason
2020-12-09 15:52     ` [PATCH v3 4/5] pretty format %(trailers): add a "keyonly" Ævar Arnfjörð Bjarmason
2020-12-09 15:52     ` [PATCH v3 5/5] pretty format %(trailers): add a "key_value_separator" Ævar Arnfjörð Bjarmason
2020-12-06  0:24   ` [PATCH v2 1/5] pretty format %(trailers) test: split a long line Ævar Arnfjörð Bjarmason
2020-12-06  0:24   ` [PATCH v2 2/5] pretty format %(trailers) doc: avoid repetition Ævar Arnfjörð Bjarmason
2020-12-07  9:09     ` Christian Couder
2020-12-06  0:24   ` [PATCH v2 3/5] pretty-format %(trailers): fix broken standalone "valueonly" Ævar Arnfjörð Bjarmason
2020-12-06  0:24   ` [PATCH v2 4/5] pretty format %(trailers): add a "keyonly" Ævar Arnfjörð Bjarmason
2020-12-07  9:17     ` Christian Couder
2020-12-06  0:24   ` [PATCH v2 5/5] pretty format %(trailers): add a "key_value_separator" Ævar Arnfjörð Bjarmason
2020-12-05  1:39 ` [PATCH 1/5] pretty format %(trailers) test: split a long line Ævar Arnfjörð Bjarmason
2020-12-05  1:39 ` [PATCH 2/5] pretty format %(trailers): avoid needless repetition Ævar Arnfjörð Bjarmason
2020-12-05  5:43   ` Christian Couder
2020-12-05  1:39 ` [PATCH 3/5] pretty format %(trailers): add a "keyonly" Ævar Arnfjörð Bjarmason
2020-12-05  6:11   ` Christian Couder
2020-12-05 12:26     ` Ævar Arnfjörð Bjarmason
2020-12-05  1:39 ` [PATCH 4/5] pretty-format %(trailers): fix broken standalone "valueonly" Ævar Arnfjörð Bjarmason
2020-12-05  6:46   ` Christian Couder
2020-12-05  1:39 ` [PATCH 5/5] pretty format %(trailers): add a "key_value_separator" Ævar Arnfjörð Bjarmason
2020-12-05  7:13   ` Christian Couder
2020-12-05  8:49     ` Ævar Arnfjörð Bjarmason

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20201025212652.3003036-8-anders@0x63.nu \
    --to=anders@0x63.nu \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=jonathantanmy@google.com \
    --cc=peff@peff.net \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).