@matheusbsbs / matheus-bagliano / commits / e814c39c2f

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 <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Jun 18, 2014 at 15:51 UTC e814c39c2fe7cc915ba70c0aa6f03156a28920fc
1 file changed +7 -13
fast-import.c
+7 -13
@@ -2269,7 +2269,7 @@ static uintmax_t parse_mark_ref_space(const char **p)
2269 char *end;
2270
2271 mark = parse_mark_ref(*p, &end);
2272 - if (*end != ' ')
2272 + if (*end++ != ' ')
2273 die("Missing space after mark: %s", command_buf.buf);
2274 *p = end;
2275 return mark;
@@ -2304,20 +2304,17 @@ static void file_change_m(const char *p, struct branch *b)
2304 if (*p == ':') {
2305 oe = find_mark(parse_mark_ref_space(&p));
2306 hashcpy(sha1, oe->idx.sha1);
2307 - } else if (starts_with(p, "inline ")) {
2307 + } else if (skip_prefix(p, "inline ", &p)) {
2308 inline_data = 1;
2309 oe = NULL; /* not used with inline_data, but makes gcc happy */
2310 - p += strlen("inline"); /* advance to space */
2310 } else {
2311 if (get_sha1_hex(p, sha1))
2312 die("Invalid dataref: %s", command_buf.buf);
2313 oe = find_object(sha1);
2314 p += 40;
2316 - if (*p != ' ')
2315 + if (*p++ != ' ')
2316 die("Missing space after SHA1: %s", command_buf.buf);
2317 }
2319 - assert(*p == ' ');
2320 - p++; /* skip space */
2318
2319 strbuf_reset(&uq);
2320 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
2471 if (*p == ':') {
2472 oe = find_mark(parse_mark_ref_space(&p));
2473 hashcpy(sha1, oe->idx.sha1);
2477 - } else if (starts_with(p, "inline ")) {
2474 + } else if (skip_prefix(p, "inline ", &p)) {
2475 inline_data = 1;
2476 oe = NULL; /* not used with inline_data, but makes gcc happy */
2480 - p += strlen("inline"); /* advance to space */
2477 } else {
2478 if (get_sha1_hex(p, sha1))
2479 die("Invalid dataref: %s", command_buf.buf);
2480 oe = find_object(sha1);
2481 p += 40;
2486 - if (*p != ' ')
2482 + if (*p++ != ' ')
2483 die("Missing space after SHA1: %s", command_buf.buf);
2484 }
2489 - assert(*p == ' ');
2490 - p++; /* skip space */
2485
2486 /* <commit-ish> */
2487 s = lookup_branch(p);
@@ -3003,6 +2997,8 @@ static struct object_entry *parse_treeish_dataref(const char **p)
2997 die("Invalid dataref: %s", command_buf.buf);
2998 e = find_object(sha1);
2999 *p += 40;
3000 + if (*(*p)++ != ' ')
3001 + die("Missing space after tree-ish: %s", command_buf.buf);
3002 }
3003
3004 while (!e || e->type != OBJ_TREE)
@@ -3054,8 +3050,6 @@ static void parse_ls(const char *p, struct branch *b)
3050 if (!is_null_sha1(root->versions[1].sha1))
3051 root->versions[1].mode = S_IFDIR;
3052 load_tree(root);
3057 - if (*p++ != ' ')
3058 - die("Missing space after tree-ish: %s", command_buf.buf);
3053 }
3054 if (*p == '"') {
3055 static struct strbuf uq = STRBUF_INIT;