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

[PATCH v3 07/27] update-ref --stdin: read the whole input at once

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
Apr 7, 2014, 13:47 UTC
Message-ID
<1396878498-19887-8-git-send-email-mhagger@alum.mit.edu>
In-Reply-To
<1396878498-19887-1-git-send-email-mhagger@alum.mit.edu>

Read the whole input into a strbuf at once, and then parse it from there. This might also be a tad faster, but that is not the point. The point is to decouple the parsing code from the input source (the old parsing code had to read new data even in the middle of commands). Add docstrings for the parsing functions.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 builtin/update-ref.c | 170 ++++++++++++++++++++++++++++++++-------------------
 1 file changed, 108 insertions(+), 62 deletions(-)
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index a8a68e8..5f197fe 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -85,44 +85,70 @@ static const char *parse_arg(const char *next, struct strbuf *arg)
 	return next;
 }
 
-static const char *parse_first_arg(const char *next, struct strbuf *arg)
+/*
+ * Parse the argument immediately after "command SP".  If not -z, then
+ * handle C-quoting.  Write the argument to arg.  Set *next to point
+ * at the character that terminates the argument.  Die if C-quoting is
+ * malformed.
+ */
+static void parse_first_arg(struct strbuf *input, const char **next,
+			    struct strbuf *arg)
 {
-	/* Parse argument immediately after "command SP" */
 	strbuf_reset(arg);
 	if (line_termination) {
 		/* Without -z, use the next argument */
-		next = parse_arg(next, arg);
+		*next = parse_arg(*next, arg);
 	} else {
-		/* With -z, use rest of first NUL-terminated line */
-		strbuf_addstr(arg, next);
-		next = next + arg->len;
+		/* With -z, use everything up to the next NUL */
+		strbuf_addstr(arg, *next);
+		*next += arg->len;
 	}
-	return next;
 }
 
-static const char *parse_next_arg(const char *next, struct strbuf *arg)
+/*
+ * Parse a SP/NUL separator followed by the next SP- or NUL-terminated
+ * argument, if any.  If there is an argument, write it to arg, set
+ * *next to point at the character terminating the argument, and
+ * return 0.  If there is no argument at all (not even the empty
+ * string), return a non-zero result and leave *next unchanged.
+ */
+static int parse_next_arg(struct strbuf *input, const char **next,
+			  struct strbuf *arg)
 {
-	/* Parse next SP-terminated or NUL-terminated argument, if any */
 	strbuf_reset(arg);
 	if (line_termination) {
 		/* Without -z, consume SP and use next argument */
-		if (!*next)
-			return NULL;
-		if (*next != ' ')
-			die("expected SP but got: %s", next);
-		next = parse_arg(next + 1, arg);
+		if (!**next || **next == line_termination)
+			return -1;
+		if (**next != ' ')
+			die("expected SP but got: %s", *next);
+		(*next)++;
+		*next = parse_arg(*next, arg);
 	} else {
 		/* With -z, read the next NUL-terminated line */
-		if (*next)
-			die("expected NUL but got: %s", next);
-		if (strbuf_getline(arg, stdin, '\0') == EOF)
-			return NULL;
-		next = arg->buf + arg->len;
+		if (**next)
+			die("expected NUL but got: %s", *next);
+		(*next)++;
+		if (*next == input->buf + input->len)
+			return -1;
+		strbuf_addstr(arg, *next);
+		*next += arg->len;
 	}
-	return next;
+	return 0;
 }
 
-static void parse_cmd_update(const char *next)
+
+/*
+ * The following five parse_cmd_*() functions parse the corresponding
+ * command.  In each case, next points at the character following the
+ * command name and the following space.  They each return a pointer
+ * to the character terminating the command, and die with an
+ * explanatory message if there are any parsing problems.  All of
+ * these functions handle either text or binary format input,
+ * depending on how line_termination is set.
+ */
+
+static const char *parse_cmd_update(struct strbuf *input, const char *next)
 {
 	struct strbuf ref = STRBUF_INIT;
 	struct strbuf newvalue = STRBUF_INIT;
@@ -131,26 +157,28 @@ static void parse_cmd_update(const char *next)
 
 	update = update_alloc();
 
-	if ((next = parse_first_arg(next, &ref)) != NULL && ref.buf[0])
+	parse_first_arg(input, &next, &ref);
+	if (ref.buf[0])
 		update_store_ref_name(update, ref.buf);
 	else
 		die("update line missing <ref>");
 
-	if ((next = parse_next_arg(next, &newvalue)) != NULL)
+	if (!parse_next_arg(input, &next, &newvalue))
 		update_store_new_sha1(update, newvalue.buf);
 	else
 		die("update %s missing <newvalue>", ref.buf);
 
-	if ((next = parse_next_arg(next, &oldvalue)) != NULL)
+	if (!parse_next_arg(input, &next, &oldvalue)) {
 		update_store_old_sha1(update, oldvalue.buf);
-	else if(!line_termination)
+		if (*next != line_termination)
+			die("update %s has extra input: %s", ref.buf, next);
+	} else if (!line_termination)
 		die("update %s missing [<oldvalue>] NUL", ref.buf);
 
-	if (next && *next)
-		die("update %s has extra input: %s", ref.buf, next);
+	return next;
 }
 
-static void parse_cmd_create(const char *next)
+static const char *parse_cmd_create(struct strbuf *input, const char *next)
 {
 	struct strbuf ref = STRBUF_INIT;
 	struct strbuf newvalue = STRBUF_INIT;
@@ -158,23 +186,27 @@ static void parse_cmd_create(const char *next)
 
 	update = update_alloc();
 
-	if ((next = parse_first_arg(next, &ref)) != NULL && ref.buf[0])
+	parse_first_arg(input, &next, &ref);
+	if (ref.buf[0])
 		update_store_ref_name(update, ref.buf);
 	else
 		die("create line missing <ref>");
 
-	if ((next = parse_next_arg(next, &newvalue)) != NULL)
+	if (!parse_next_arg(input, &next, &newvalue))
 		update_store_new_sha1(update, newvalue.buf);
 	else
 		die("create %s missing <newvalue>", ref.buf);
+
 	if (is_null_sha1(update->new_sha1))
 		die("create %s given zero new value", ref.buf);
 
-	if (next && *next)
+	if (*next != line_termination)
 		die("create %s has extra input: %s", ref.buf, next);
+
+	return next;
 }
 
-static void parse_cmd_delete(const char *next)
+static const char *parse_cmd_delete(struct strbuf *input, const char *next)
 {
 	struct strbuf ref = STRBUF_INIT;
 	struct strbuf oldvalue = STRBUF_INIT;
@@ -182,23 +214,26 @@ static void parse_cmd_delete(const char *next)
 
 	update = update_alloc();
 
-	if ((next = parse_first_arg(next, &ref)) != NULL && ref.buf[0])
+	parse_first_arg(input, &next, &ref);
+	if (ref.buf[0])
 		update_store_ref_name(update, ref.buf);
 	else
 		die("delete line missing <ref>");
 
-	if ((next = parse_next_arg(next, &oldvalue)) != NULL)
+	if (!parse_next_arg(input, &next, &oldvalue)) {
 		update_store_old_sha1(update, oldvalue.buf);
-	else if(!line_termination)
+		if (update->have_old && is_null_sha1(update->old_sha1))
+			die("delete %s given zero old value", ref.buf);
+	} else if (!line_termination)
 		die("delete %s missing [<oldvalue>] NUL", ref.buf);
-	if (update->have_old && is_null_sha1(update->old_sha1))
-		die("delete %s given zero old value", ref.buf);
 
-	if (next && *next)
+	if (*next != line_termination)
 		die("delete %s has extra input: %s", ref.buf, next);
+
+	return next;
 }
 
-static void parse_cmd_verify(const char *next)
+static const char *parse_cmd_verify(struct strbuf *input, const char *next)
 {
 	struct strbuf ref = STRBUF_INIT;
 	struct strbuf value = STRBUF_INIT;
@@ -206,53 +241,64 @@ static void parse_cmd_verify(const char *next)
 
 	update = update_alloc();
 
-	if ((next = parse_first_arg(next, &ref)) != NULL && ref.buf[0])
+	parse_first_arg(input, &next, &ref);
+	if (ref.buf[0])
 		update_store_ref_name(update, ref.buf);
 	else
 		die("verify line missing <ref>");
 
-	if ((next = parse_next_arg(next, &value)) != NULL) {
+	if (!parse_next_arg(input, &next, &value)) {
 		update_store_old_sha1(update, value.buf);
 		update_store_new_sha1(update, value.buf);
-	} else if(!line_termination)
+	} else if (!line_termination)
 		die("verify %s missing [<oldvalue>] NUL", ref.buf);
 
-	if (next && *next)
+	if (*next != line_termination)
 		die("verify %s has extra input: %s", ref.buf, next);
+
+	return next;
 }
 
-static void parse_cmd_option(const char *next)
+static const char *parse_cmd_option(struct strbuf *input, const char *next)
 {
-	if (!strcmp(next, "no-deref"))
+	if (!strncmp(next, "no-deref", 8) && next[8] == line_termination)
 		update_flags |= REF_NODEREF;
 	else
 		die("option unknown: %s", next);
+	return next + 8;
 }
 
 static void update_refs_stdin(void)
 {
-	struct strbuf cmd = STRBUF_INIT;
+	struct strbuf input = STRBUF_INIT;
+	const char *next;
 
+	if (strbuf_read(&input, 0, 1000) < 0)
+		die_errno("could not read from stdin");
+	next = input.buf;
 	/* Read each line dispatch its command */
-	while (strbuf_getline(&cmd, stdin, line_termination) != EOF)
-		if (!cmd.buf[0])
+	while (next < input.buf + input.len) {
+		if (*next == line_termination)
 			die("empty command in input");
-		else if (isspace(*cmd.buf))
-			die("whitespace before command: %s", cmd.buf);
-		else if (starts_with(cmd.buf, "update "))
-			parse_cmd_update(cmd.buf + 7);
-		else if (starts_with(cmd.buf, "create "))
-			parse_cmd_create(cmd.buf + 7);
-		else if (starts_with(cmd.buf, "delete "))
-			parse_cmd_delete(cmd.buf + 7);
-		else if (starts_with(cmd.buf, "verify "))
-			parse_cmd_verify(cmd.buf + 7);
-		else if (starts_with(cmd.buf, "option "))
-			parse_cmd_option(cmd.buf + 7);
+		else if (isspace(*next))
+			die("whitespace before command: %s", next);
+		else if (starts_with(next, "update "))
+			next = parse_cmd_update(&input, next + 7);
+		else if (starts_with(next, "create "))
+			next = parse_cmd_create(&input, next + 7);
+		else if (starts_with(next, "delete "))
+			next = parse_cmd_delete(&input, next + 7);
+		else if (starts_with(next, "verify "))
+			next = parse_cmd_verify(&input, next + 7);
+		else if (starts_with(next, "option "))
+			next = parse_cmd_option(&input, next + 7);
 		else
-			die("unknown command: %s", cmd.buf);
+			die("unknown command: %s", next);
+
+		next++;
+	}
 
-	strbuf_release(&cmd);
+	strbuf_release(&input);
 }
 
 int cmd_update_ref(int argc, const char **argv, const char *prefix)

-- 
1.9.1
Previous: Michael HaggertyNext: Michael Haggerty
Message 8 of 34 in “Clean up update-refs --stdin and implement ref_transaction”
  1. 00/27 Clean up update-refs --stdin and implement ref_transactionMichael Haggerty, Apr 7, 2014
  2. 01/27 t1400: fix name and expected result of one testMichael Haggerty, Apr 7, 2014
  3. 02/27 t1400: provide more usual input to the commandMichael Haggerty, Apr 7, 2014
  4. 03/27 parse_arg(): really test that argument is properly terminatedMichael Haggerty, Apr 7, 2014
  5. 04/27 t1400: add some more tests involving quoted argumentsMichael Haggerty, Apr 7, 2014
  6. 05/27 refs.h: rename the action_on_err constantsMichael Haggerty, Apr 7, 2014
  7. 06/27 update_refs(): fix constnessMichael Haggerty, Apr 7, 2014
  8. 07/27 update-ref --stdin: read the whole input at onceMichael Haggerty, Apr 7, 2014
  9. 08/27 parse_cmd_verify(): copy old_sha1 instead of evaluating <oldvalue> twiceMichael Haggerty, Apr 7, 2014
  10. 09/27 update-ref.c: extract a new function, parse_refname()Michael Haggerty, Apr 7, 2014
  11. 10/27 update-ref --stdin: improve error messages for invalid valuesMichael Haggerty, Apr 7, 2014
  12. 11/27 update-ref --stdin: make error messages more consistentMichael Haggerty, Apr 7, 2014
  13. 12/27 update-ref --stdin: simplify error messages for missing oldvaluesMichael Haggerty, Apr 7, 2014
  14. 13/27 t1400: test that stdin -z update treats empty <newvalue> as zerosMichael Haggerty, Apr 7, 2014
  15. 14/27 update-ref.c: extract a new function, parse_next_sha1()Michael Haggerty, Apr 7, 2014
  16. 15/27 update-ref --stdin -z: deprecate interpreting the empty string as zerosMichael Haggerty, Apr 7, 2014
  17. 16/27 t1400: test one mistake at a timeMichael Haggerty, Apr 7, 2014
  18. 17/27 update-ref --stdin: improve the error message for unexpected EOFMichael Haggerty, Apr 7, 2014
  19. 18/27 update-ref --stdin: harmonize error messagesMichael Haggerty, Apr 7, 2014
  20. 19/27 refs: add a concept of a reference transactionMichael Haggerty, Apr 7, 2014
  21. Junio C HamanoApr 7, 2014
  22. Michael HaggertyApr 14, 2014
  23. Junio C HamanoApr 14, 2014
  24. Michael HaggertyApr 15, 2014
  25. Junio C HamanoApr 15, 2014
  26. 20/27 update-ref --stdin: reimplement using reference transactionsMichael Haggerty, Apr 7, 2014
  27. 21/27 refs: remove API function update_refs()Michael Haggerty, Apr 7, 2014
  28. 22/27 struct ref_update: rename field "ref_name" to "refname"Michael Haggerty, Apr 7, 2014
  29. 23/27 struct ref_update: store refname as a FLEX_ARRAYMichael Haggerty, Apr 7, 2014
  30. 24/27 ref_transaction_commit(): simplify code using temporary variablesMichael Haggerty, Apr 7, 2014
  31. 25/27 struct ref_update: add a lock fieldMichael Haggerty, Apr 7, 2014
  32. 26/27 struct ref_update: add a type fieldMichael Haggerty, Apr 7, 2014
  33. 27/27 ref_transaction_commit(): work with transaction->updates in placeMichael Haggerty, Apr 7, 2014
  34. Ronnie SahlbergApr 11, 2014

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.