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

[PATCH v4 5/8] trailer: clarify failure modes in parse_trailer

From
JTJonathan Tan <jonathantanmy@google.com>
Date
Oct 20, 2016, 21:39 UTC
Message-ID
<8aa456a551bf5cf44cea5ea7e33f32e190c45658.1476998988.git.jonathantanmy@google.com>
In-Reply-To
<cover.1476998988.git.jonathantanmy@google.com>

The parse_trailer function has a few modes of operation, all depending on whether the separator is present in its input, and if yes, the separator's position. Some of these modes are failure modes, and these failure modes are handled differently depending on whether the trailer line was sourced from a file or from a command-line argument.

Extract a function to find the separator, allowing the invokers of parse_trailer to determine how to handle the failure modes instead of making parse_trailer do it.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
 trailer.c | 70 +++++++++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 48 insertions(+), 22 deletions(-)
diff --git a/trailer.c b/trailer.c
index 99018f8..137a3fb 100644
--- a/trailer.c
+++ b/trailer.c
@@ -543,29 +543,40 @@ static int token_matches_item(const char *tok, struct arg_item *item, int tok_le
 	return item->conf.key ? !strncasecmp(tok, item->conf.key, tok_len) : 0;
 }
 
-static int parse_trailer(struct strbuf *tok, struct strbuf *val,
-			 const struct conf_info **conf, const char *trailer)
+/*
+ * Return the location of the first separator or '=' in line, or -1 if either a
+ * newline or the null terminator is reached first.
+ */
+static int find_separator(const char *line)
+{
+	const char *c;
+	for (c = line; ; c++) {
+		if (!*c || *c == '\n')
+			return -1;
+		if (*c == '=' || strchr(separators, *c))
+			return c - line;
+	}
+}
+
+/*
+ * Obtain the token, value, and conf from the given trailer.
+ *
+ * separator_pos must not be 0, since the token cannot be an empty string.
+ *
+ * If separator_pos is -1, interpret the whole trailer as a token.
+ */
+static void parse_trailer(struct strbuf *tok, struct strbuf *val,
+			 const struct conf_info **conf, const char *trailer,
+			 int separator_pos)
 {
-	size_t len;
-	struct strbuf seps = STRBUF_INIT;
 	struct arg_item *item;
 	int tok_len;
 	struct list_head *pos;
 
-	strbuf_addstr(&seps, separators);
-	strbuf_addch(&seps, '=');
-	len = strcspn(trailer, seps.buf);
-	strbuf_release(&seps);
-	if (len == 0) {
-		int l = strlen(trailer);
-		while (l > 0 && isspace(trailer[l - 1]))
-			l--;
-		return error(_("empty trailer token in trailer '%.*s'"), l, trailer);
-	}
-	if (len < strlen(trailer)) {
-		strbuf_add(tok, trailer, len);
+	if (separator_pos != -1) {
+		strbuf_add(tok, trailer, separator_pos);
 		strbuf_trim(tok);
-		strbuf_addstr(val, trailer + len + 1);
+		strbuf_addstr(val, trailer + separator_pos + 1);
 		strbuf_trim(val);
 	} else {
 		strbuf_addstr(tok, trailer);
@@ -587,8 +598,6 @@ static int parse_trailer(struct strbuf *tok, struct strbuf *val,
 			break;
 		}
 	}
-
-	return 0;
 }
 
 static void add_trailer_item(struct list_head *head, char *tok, char *val)
@@ -631,11 +640,22 @@ static void process_command_line_args(struct list_head *arg_head,
 
 	/* Add an arg item for each trailer on the command line */
 	for_each_string_list_item(tr, trailers) {
-		if (!parse_trailer(&tok, &val, &conf, tr->string))
+		int separator_pos = find_separator(tr->string);
+		if (separator_pos == 0) {
+			struct strbuf sb = STRBUF_INIT;
+			strbuf_addstr(&sb, tr->string);
+			strbuf_trim(&sb);
+			error(_("empty trailer token in trailer '%.*s'"),
+			      (int) sb.len, sb.buf);
+			strbuf_release(&sb);
+		} else {
+			parse_trailer(&tok, &val, &conf, tr->string,
+				      separator_pos);
 			add_arg_item(arg_head,
 				     strbuf_detach(&tok, NULL),
 				     strbuf_detach(&val, NULL),
 				     conf);
+		}
 	}
 }
 
@@ -775,11 +795,17 @@ static int process_input_file(FILE *outfile,
 
 	/* Parse trailer lines */
 	for (i = trailer_start; i < trailer_end; i++) {
-		if (lines[i]->buf[0] != comment_line_char &&
-		    !parse_trailer(&tok, &val, NULL, lines[i]->buf))
+		int separator_pos;
+		if (lines[i]->buf[0] == comment_line_char)
+			continue;
+		separator_pos = find_separator(lines[i]->buf);
+		if (separator_pos >= 1) {
+			parse_trailer(&tok, &val, NULL, lines[i]->buf,
+				      separator_pos);
 			add_trailer_item(head,
 					 strbuf_detach(&tok, NULL),
 					 strbuf_detach(&val, NULL));
+		}
 	}
 
 	return trailer_end;
-- 
2.8.0.rc3.226.g39d4020
Previous: Jonathan TanNext: Stefan Beller
Message 5 of 19 in “allow non-trailers and multiple-line trailers”
  1. 0/8 allow non-trailers and multiple-line trailersJonathan Tan, Oct 20, 2016
  2. 1/8 trailer: improve const correctnessJonathan Tan, Oct 20, 2016
  3. 2/8 trailer: use list.h for doubly-linked listJonathan Tan, Oct 20, 2016
  4. 6/8 trailer: allow non-trailers in trailer blockJonathan Tan, Oct 20, 2016
  5. 5/8 trailer: clarify failure modes in parse_trailerJonathan Tan, Oct 20, 2016
  6. Stefan BellerOct 20, 2016
  7. Junio C HamanoOct 20, 2016
  8. Jonathan TanOct 20, 2016
  9. Junio C HamanoOct 20, 2016
  10. Jonathan TanOct 20, 2016
  11. Junio C HamanoOct 21, 2016
  12. Christian CouderOct 22, 2016
  13. Junio C HamanoOct 22, 2016
  14. Christian CouderOct 22, 2016
  15. Jonathan TanOct 20, 2016
  16. 8/8 trailer: support values folded to multiple linesJonathan Tan, Oct 20, 2016
  17. 4/8 trailer: make args have their own structJonathan Tan, Oct 20, 2016
  18. 7/8 trailer: forbid leading whitespace in trailersJonathan Tan, Oct 20, 2016
  19. 3/8 trailer: streamline trailer item create and addJonathan Tan, Oct 20, 2016

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.