From e814c39c2fe7cc915ba70c0aa6f03156a28920fc Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 18 Jun 2014 15:51:57 -0400 Subject: [PATCH] fast-import: refactor parsing of spaces When we see a file change in a commit, we expect one of: 1. A mark. 2. An "inline" keyword. 3. An object sha1. The handling of spaces is inconsistent between the three options. Option 1 calls a sub-function which checks for the space, but doesn't parse past it. Option 2 parses the space, then deliberately avoids moving the pointer past it. Option 3 detects the space locally but doesn't move past it. This is confusing, because it looks like option 1 forgets to check for the space (it's just buried). And option 2 checks for "inline ", but only moves strlen("inline") characters forward, which looks like a bug but isn't. We can make this more clear by just having each branch move past the space as it is checked (and we can replace the doubled use of "inline" with a call to skip_prefix). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- fast-import.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/fast-import.c b/fast-import.c index c608fc68a0..fa635f75c3 100644 --- a/fast-import.c +++ b/fast-import.c @@ -2269,7 +2269,7 @@ static uintmax_t parse_mark_ref_space(const char **p) char *end; mark = parse_mark_ref(*p, &end); - if (*end != ' ') + if (*end++ != ' ') die("Missing space after mark: %s", command_buf.buf); *p = end; return mark; @@ -2304,20 +2304,17 @@ static void file_change_m(const char *p, struct branch *b) if (*p == ':') { oe = find_mark(parse_mark_ref_space(&p)); hashcpy(sha1, oe->idx.sha1); - } else if (starts_with(p, "inline ")) { + } else if (skip_prefix(p, "inline ", &p)) { inline_data = 1; oe = NULL; /* not used with inline_data, but makes gcc happy */ - p += strlen("inline"); /* advance to space */ } else { if (get_sha1_hex(p, sha1)) die("Invalid dataref: %s", command_buf.buf); oe = find_object(sha1); p += 40; - if (*p != ' ') + if (*p++ != ' ') die("Missing space after SHA1: %s", command_buf.buf); } - assert(*p == ' '); - p++; /* skip space */ strbuf_reset(&uq); if (!unquote_c_style(&uq, p, &endp)) { @@ -2474,20 +2471,17 @@ static void note_change_n(const char *p, struct branch *b, unsigned char *old_fa if (*p == ':') { oe = find_mark(parse_mark_ref_space(&p)); hashcpy(sha1, oe->idx.sha1); - } else if (starts_with(p, "inline ")) { + } else if (skip_prefix(p, "inline ", &p)) { inline_data = 1; oe = NULL; /* not used with inline_data, but makes gcc happy */ - p += strlen("inline"); /* advance to space */ } else { if (get_sha1_hex(p, sha1)) die("Invalid dataref: %s", command_buf.buf); oe = find_object(sha1); p += 40; - if (*p != ' ') + if (*p++ != ' ') die("Missing space after SHA1: %s", command_buf.buf); } - assert(*p == ' '); - p++; /* skip space */ /* */ s = lookup_branch(p); @@ -3003,6 +2997,8 @@ static struct object_entry *parse_treeish_dataref(const char **p) die("Invalid dataref: %s", command_buf.buf); e = find_object(sha1); *p += 40; + if (*(*p)++ != ' ') + die("Missing space after tree-ish: %s", command_buf.buf); } while (!e || e->type != OBJ_TREE) @@ -3054,8 +3050,6 @@ static void parse_ls(const char *p, struct branch *b) if (!is_null_sha1(root->versions[1].sha1)) root->versions[1].mode = S_IFDIR; load_tree(root); - if (*p++ != ' ') - die("Missing space after tree-ish: %s", command_buf.buf); } if (*p == '"') { static struct strbuf uq = STRBUF_INIT;