Skip to content

Commit

Permalink
fast-import: refactor parsing of spaces
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Jeff King authored and Junio C Hamano committed Jun 20, 2014
1 parent 0539cc0 commit e814c39
Showing 1 changed file with 7 additions and 13 deletions.
20 changes: 7 additions & 13 deletions fast-import.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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 */

/* <commit-ish> */
s = lookup_branch(p);
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit e814c39

Please sign in to comment.