[PATCH 6/8] pretty: refactor parsing of line-wrapping "%w" placeholder

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

 



Our parsing of a "%w" placeholder is quite a big chunk of code in
the middle of our switch for handling a few different placeholders. We
parse into three different variables, then use them to compare to and
update existing values in the big `struct format_commit_context`.

Pull out a helper function for parsing such a "%w" placeholder. Define a
struct for collecting the three variables.

Unlike recent commits, parsing and subsequent use are already a bit more
separated in the sense that we don't parse directly into the big context
struct. Thus, unlike the preceding commits, this does not fix any bugs
that I'm aware of. There's still value in separating parsing and usage
more clearly and simplifying `format_commit_one()`.

Note that we use two different types for these values, `unsigned long`
when parsing, `size_t` when eventually applying. Let's go for `size_t`
in our struct. I don't know if there are platforms where assigning an
`unsigned long` to a `size_t` could truncate the value, but since we
already verify the values to be at most 16 KiB, we should be able to fit
them into any sane `size_t`s.

Signed-off-by: Martin Ågren <martin.agren@xxxxxxxxx>
---
 pretty.c | 120 +++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 76 insertions(+), 44 deletions(-)

diff --git a/pretty.c b/pretty.c
index f53e77ed86..c44ff87481 100644
--- a/pretty.c
+++ b/pretty.c
@@ -893,6 +893,10 @@ struct padding_args {
 	int padding;
 };
 
+struct rewrap_args {
+	size_t width, indent1, indent2;
+};
+
 struct format_commit_context {
 	struct repository *repository;
 	const struct commit *commit;
@@ -902,7 +906,7 @@ struct format_commit_context {
 	struct signature_check signature_check;
 	const char *message;
 	char *commit_encoding;
-	size_t width, indent1, indent2;
+	struct rewrap_args rewrap;
 	int auto_color;
 	struct padding_args pad;
 
@@ -1034,18 +1038,21 @@ static void strbuf_wrap(struct strbuf *sb, size_t pos,
 
 static void rewrap_message_tail(struct strbuf *sb,
 				struct format_commit_context *c,
-				size_t new_width, size_t new_indent1,
-				size_t new_indent2)
+				const struct rewrap_args *new_rewrap)
 {
-	if (c->width == new_width && c->indent1 == new_indent1 &&
-	    c->indent2 == new_indent2)
+	const struct rewrap_args *old_rewrap = &c->rewrap;
+
+	if (old_rewrap->width == new_rewrap->width &&
+	    old_rewrap->indent1 == new_rewrap->indent1 &&
+	    old_rewrap->indent2 == new_rewrap->indent2)
 		return;
+
 	if (c->wrap_start < sb->len)
-		strbuf_wrap(sb, c->wrap_start, c->width, c->indent1, c->indent2);
+		strbuf_wrap(sb, c->wrap_start, old_rewrap->width,
+			    old_rewrap->indent1, old_rewrap->indent2);
+
 	c->wrap_start = sb->len;
-	c->width = new_width;
-	c->indent1 = new_indent1;
-	c->indent2 = new_indent2;
+	c->rewrap = *new_rewrap;
 }
 
 static int format_reflog_person(struct strbuf *sb,
@@ -1443,6 +1450,57 @@ static void free_decoration_options(const struct decoration_options *opts)
 	free(opts->tag);
 }
 
+static size_t parse_rewrap(const char *placeholder, struct rewrap_args *rewrap)
+{
+	unsigned long width = 0, indent1 = 0, indent2 = 0;
+	char *next;
+	const char *start;
+	const char *end;
+
+	memset(rewrap, 0, sizeof(*rewrap));
+
+	if (placeholder[1] != '(')
+		return 0;
+
+	start = placeholder + 2;
+	end = strchr(start, ')');
+
+	if (!end)
+		return 0;
+	if (end > start) {
+		width = strtoul(start, &next, 10);
+		if (*next == ',') {
+			indent1 = strtoul(next + 1, &next, 10);
+			if (*next == ',') {
+				indent2 = strtoul(next + 1,
+						 &next, 10);
+			}
+		}
+		if (*next != ')')
+			return 0;
+	}
+
+	/*
+	 * We need to limit the format here as it allows the
+	 * user to prepend arbitrarily many bytes to the buffer
+	 * when rewrapping.
+	 */
+	if (width > FORMATTING_LIMIT ||
+	    indent1 > FORMATTING_LIMIT ||
+	    indent2 > FORMATTING_LIMIT)
+		return 0;
+
+	/*
+	 * These values are small enough to fit in any
+	 * real-world size_t.
+	 */
+	rewrap->width = width;
+	rewrap->indent1 = indent1;
+	rewrap->indent2 = indent2;
+
+	return end - placeholder + 1;
+}
+
 static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 				const char *placeholder,
 				struct format_commit_context *c)
@@ -1478,40 +1536,13 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 			return ret;
 		}
 	case 'w':
-		if (placeholder[1] == '(') {
-			unsigned long width = 0, indent1 = 0, indent2 = 0;
-			char *next;
-			const char *start = placeholder + 2;
-			const char *end = strchr(start, ')');
-			if (!end)
-				return 0;
-			if (end > start) {
-				width = strtoul(start, &next, 10);
-				if (*next == ',') {
-					indent1 = strtoul(next + 1, &next, 10);
-					if (*next == ',') {
-						indent2 = strtoul(next + 1,
-								 &next, 10);
-					}
-				}
-				if (*next != ')')
-					return 0;
-			}
-
-			/*
-			 * We need to limit the format here as it allows the
-			 * user to prepend arbitrarily many bytes to the buffer
-			 * when rewrapping.
-			 */
-			if (width > FORMATTING_LIMIT ||
-			    indent1 > FORMATTING_LIMIT ||
-			    indent2 > FORMATTING_LIMIT)
-				return 0;
-			rewrap_message_tail(sb, c, width, indent1, indent2);
-			return end - placeholder + 1;
-		} else
-			return 0;
-
+		{
+			struct rewrap_args rewrap;
+			res = parse_rewrap(placeholder, &rewrap);
+			if (res)
+				rewrap_message_tail(sb, c, &rewrap);
+			return res;
+		}
 	case '<':
 	case '>':
 		return parse_padding_placeholder(placeholder, &c->pad);
@@ -2005,6 +2036,7 @@ void repo_format_commit_message(struct repository *r,
 	};
 	const char *output_enc = pretty_ctx->output_encoding;
 	const char *utf8 = "UTF-8";
+	const struct rewrap_args rewrap_reset = { 0 };
 
 	while (strbuf_expand_step(sb, &format)) {
 		size_t len;
@@ -2016,7 +2048,7 @@ void repo_format_commit_message(struct repository *r,
 		else
 			strbuf_addch(sb, '%');
 	}
-	rewrap_message_tail(sb, &context, 0, 0, 0);
+	rewrap_message_tail(sb, &context, &rewrap_reset);
 
 	/*
 	 * Convert output to an actual output encoding; note that
-- 
2.49.0.472.ge94155a9ec





[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