@matheusbsbs / matheus-bagliano / commits / 97313bef2a

fast-import: use skip_prefix for parsing input

Fast-import does a lot of parsing of commands and dispatching to sub-functions. For example, given "option foo", we might recognize "option " using starts_with, and then hand it off to parse_option() to do the rest. However, we do not let parse_option know that we have parsed the first part already. It gets the full buffer, and has to skip past the uninteresting bits. Some functions simply add a magic constant: char *option = command_buf.buf + 7; Others use strlen: char *option = command_buf.buf + strlen("option "); And others use strchr: char *option = strchr(command_buf.buf, ' ') + 1; All of these are brittle and easy to get wrong (especially given that the starts_with call and the code that assumes the presence of the prefix are far apart). Instead, we can use skip_prefix, and just pass each handler a pointer to its arguments. 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:49 UTC 97313bef2a16eb8b3d40830204c20c34ba9d6554

1 file changed +51 -72

fast-import.c

+51 -72

@@ -371,8 +371,8 @@ static volatile sig_atomic_t checkpoint_requested;

371 static int cat_blob_fd = STDOUT_FILENO;

372

373 static void parse_argv(void);

374 -static void parse_cat_blob(void);

375 -static void parse_ls(struct branch *b);

374 +static void parse_cat_blob(const char *p);

375 +static void parse_ls(const char *p, struct branch *b);

376

377 static void write_branch_report(FILE *rpt, struct branch *b)

378 {

@@ -1861,6 +1861,8 @@ static int read_next_command(void)

1861 }

1862

1863 for (;;) {

1864 + const char *p;

1865 +

1866 if (unread_command_buf) {

1867 unread_command_buf = 0;

1868 } else {

@@ -1893,8 +1895,8 @@ static int read_next_command(void)

1895 rc->prev->next = rc;

1896 cmd_tail = rc;

1897 }

1896 - if (starts_with(command_buf.buf, "cat-blob ")) {

1897 - parse_cat_blob();

1898 + if (skip_prefix(command_buf.buf, "cat-blob ", &p)) {

1899 + parse_cat_blob(p);

1900 continue;

1901 }

1902 if (command_buf.buf[0] == '#')

@@ -2273,9 +2275,8 @@ static uintmax_t parse_mark_ref_space(const char **p)

2275 return mark;

2276 }

2277

2276 -static void file_change_m(struct branch *b)

2278 +static void file_change_m(const char *p, struct branch *b)

2279 {

2278 - const char *p = command_buf.buf + 2;

2280 static struct strbuf uq = STRBUF_INIT;

2281 const char *endp;

2282 struct object_entry *oe;

@@ -2376,9 +2377,8 @@ static void file_change_m(struct branch *b)

2377 tree_content_set(&b->branch_tree, p, sha1, mode, NULL);

2378 }

2379

2379 -static void file_change_d(struct branch *b)

2380 +static void file_change_d(const char *p, struct branch *b)

2381 {

2381 - const char *p = command_buf.buf + 2;

2382 static struct strbuf uq = STRBUF_INIT;

2383 const char *endp;

2384

@@ -2391,15 +2391,14 @@ static void file_change_d(struct branch *b)

2391 tree_content_remove(&b->branch_tree, p, NULL, 1);

2392 }

2393

2394 -static void file_change_cr(struct branch *b, int rename)

2394 +static void file_change_cr(const char *s, struct branch *b, int rename)

2395 {

2396 - const char *s, *d;

2396 + const char *d;

2397 static struct strbuf s_uq = STRBUF_INIT;

2398 static struct strbuf d_uq = STRBUF_INIT;

2399 const char *endp;

2400 struct tree_entry leaf;

2401

2402 - s = command_buf.buf + 2;

2402 strbuf_reset(&s_uq);

2403 if (!unquote_c_style(&s_uq, s, &endp)) {

2404 if (*endp != ' ')

@@ -2444,9 +2443,8 @@ static void file_change_cr(struct branch *b, int rename)

2443 leaf.tree);

2444 }

2445

2447 -static void note_change_n(struct branch *b, unsigned char *old_fanout)

2446 +static void note_change_n(const char *p, struct branch *b, unsigned char *old_fanout)

2447 {

2449 - const char *p = command_buf.buf + 2;

2448 static struct strbuf uq = STRBUF_INIT;

2449 struct object_entry *oe;

2450 struct branch *s;

@@ -2587,7 +2585,7 @@ static int parse_from(struct branch *b)

2585 const char *from;

2586 struct branch *s;

2587

2590 - if (!starts_with(command_buf.buf, "from "))

2588 + if (!skip_prefix(command_buf.buf, "from ", &from))

2589 return 0;

2590

2591 if (b->branch_tree.tree) {

@@ -2595,7 +2593,6 @@ static int parse_from(struct branch *b)

2593 b->branch_tree.tree = NULL;

2594 }

2595

2598 - from = strchr(command_buf.buf, ' ') + 1;

2596 s = lookup_branch(from);

2597 if (b == s)

2598 die("Can't create a branch from itself: %s", b->name);

@@ -2636,8 +2633,7 @@ static struct hash_list *parse_merge(unsigned int *count)

2633 struct branch *s;

2634

2635 *count = 0;

2639 - while (starts_with(command_buf.buf, "merge ")) {

2640 - from = strchr(command_buf.buf, ' ') + 1;

2636 + while (skip_prefix(command_buf.buf, "merge ", &from)) {

2637 n = xmalloc(sizeof(*n));

2638 s = lookup_branch(from);

2639 if (s)

@@ -2668,11 +2664,10 @@ static struct hash_list *parse_merge(unsigned int *count)

2664 return list;

2665 }

2666

2671 -static void parse_new_commit(void)

2667 +static void parse_new_commit(const char *arg)

2668 {

2669 static struct strbuf msg = STRBUF_INIT;

2670 struct branch *b;

2675 - char *sp;

2671 char *author = NULL;

2672 char *committer = NULL;

2673 struct hash_list *merge_list = NULL;

@@ -2680,11 +2675,9 @@ static void parse_new_commit(void)

2675 unsigned char prev_fanout, new_fanout;

2676 const char *v;

2677

2683 - /* Obtain the branch name from the rest of our command */

2684 - sp = strchr(command_buf.buf, ' ') + 1;

2685 - b = lookup_branch(sp);

2678 + b = lookup_branch(arg);

2679 if (!b)

2687 - b = new_branch(sp);

2680 + b = new_branch(arg);

2681

2682 read_next_command();

2683 parse_mark();

@@ -2713,20 +2706,20 @@ static void parse_new_commit(void)

2706

2707 /* file_change* */

2708 while (command_buf.len > 0) {

2716 - if (starts_with(command_buf.buf, "M "))

2717 - file_change_m(b);

2718 - else if (starts_with(command_buf.buf, "D "))

2719 - file_change_d(b);

2720 - else if (starts_with(command_buf.buf, "R "))

2721 - file_change_cr(b, 1);

2722 - else if (starts_with(command_buf.buf, "C "))

2723 - file_change_cr(b, 0);

2724 - else if (starts_with(command_buf.buf, "N "))

2725 - note_change_n(b, &prev_fanout);

2709 + if (skip_prefix(command_buf.buf, "M ", &v))

2710 + file_change_m(v, b);

2711 + else if (skip_prefix(command_buf.buf, "D ", &v))

2712 + file_change_d(v, b);

2713 + else if (skip_prefix(command_buf.buf, "R ", &v))

2714 + file_change_cr(v, b, 1);

2715 + else if (skip_prefix(command_buf.buf, "C ", &v))

2716 + file_change_cr(v, b, 0);

2717 + else if (skip_prefix(command_buf.buf, "N ", &v))

2718 + note_change_n(v, b, &prev_fanout);

2719 else if (!strcmp("deleteall", command_buf.buf))

2720 file_change_deleteall(b);

2728 - else if (starts_with(command_buf.buf, "ls "))

2729 - parse_ls(b);

2721 + else if (skip_prefix(command_buf.buf, "ls ", &v))

2722 + parse_ls(v, b);

2723 else {

2724 unread_command_buf = 1;

2725 break;

@@ -2769,10 +2762,9 @@ static void parse_new_commit(void)

2762 b->last_commit = object_count_by_type[OBJ_COMMIT];

2763 }

2764

2772 -static void parse_new_tag(void)

2765 +static void parse_new_tag(const char *arg)

2766 {

2767 static struct strbuf msg = STRBUF_INIT;

2775 - char *sp;

2768 const char *from;

2769 char *tagger;

2770 struct branch *s;

@@ -2782,11 +2774,9 @@ static void parse_new_tag(void)

2774 enum object_type type;

2775 const char *v;

2776

2785 - /* Obtain the new tag name from the rest of our command */

2786 - sp = strchr(command_buf.buf, ' ') + 1;

2777 t = pool_alloc(sizeof(struct tag));

2778 memset(t, 0, sizeof(struct tag));

2789 - t->name = pool_strdup(sp);

2779 + t->name = pool_strdup(arg);

2780 if (last_tag)

2781 last_tag->next_tag = t;

2782 else

@@ -2795,9 +2785,8 @@ static void parse_new_tag(void)

2785 read_next_command();

2786

2787 /* from ... */

2798 - if (!starts_with(command_buf.buf, "from "))

2788 + if (!skip_prefix(command_buf.buf, "from ", &from))

2789 die("Expected from command, got %s", command_buf.buf);

2800 - from = strchr(command_buf.buf, ' ') + 1;

2790 s = lookup_branch(from);

2791 if (s) {

2792 if (is_null_sha1(s->sha1))

@@ -2853,14 +2842,11 @@ static void parse_new_tag(void)

2842 t->pack_id = pack_id;

2843 }

2844

2856 -static void parse_reset_branch(void)

2845 +static void parse_reset_branch(const char *arg)

2846 {

2847 struct branch *b;

2859 - char *sp;

2848

2861 - /* Obtain the branch name from the rest of our command */

2862 - sp = strchr(command_buf.buf, ' ') + 1;

2863 - b = lookup_branch(sp);

2849 + b = lookup_branch(arg);

2850 if (b) {

2851 hashclr(b->sha1);

2852 hashclr(b->branch_tree.versions[0].sha1);

@@ -2871,7 +2857,7 @@ static void parse_reset_branch(void)

2857 }

2858 }

2859 else

2874 - b = new_branch(sp);

2860 + b = new_branch(arg);

2861 read_next_command();

2862 parse_from(b);

2863 if (command_buf.len > 0)

@@ -2929,14 +2915,12 @@ static void cat_blob(struct object_entry *oe, unsigned char sha1[20])

2915 free(buf);

2916 }

2917

2932 -static void parse_cat_blob(void)

2918 +static void parse_cat_blob(const char *p)

2919 {

2934 - const char *p;

2920 struct object_entry *oe = oe;

2921 unsigned char sha1[20];

2922

2923 /* cat-blob SP <object> LF */

2939 - p = command_buf.buf + strlen("cat-blob ");

2924 if (*p == ':') {

2925 oe = find_mark(parse_mark_ref_eol(p));

2926 if (!oe)

@@ -3053,14 +3037,12 @@ static void print_ls(int mode, const unsigned char *sha1, const char *path)

3037 cat_blob_write(line.buf, line.len);

3038 }

3039

3056 -static void parse_ls(struct branch *b)

3040 +static void parse_ls(const char *p, struct branch *b)

3041 {

3058 - const char *p;

3042 struct tree_entry *root = NULL;

3043 struct tree_entry leaf = {NULL};

3044

3045 /* ls SP (<tree-ish> SP)? <path> */

3063 - p = command_buf.buf + strlen("ls ");

3046 if (*p == '"') {

3047 if (!b)

3048 die("Not in a commit: %s", command_buf.buf);

@@ -3276,10 +3258,8 @@ static int parse_one_feature(const char *feature, int from_stream)

3258 return 1;

3259 }

3260

3279 -static void parse_feature(void)

3261 +static void parse_feature(const char *feature)

3262 {

3281 - char *feature = command_buf.buf + 8;

3282 -

3263 if (seen_data_command)

3264 die("Got feature command '%s' after data command", feature);

3265

@@ -3289,10 +3269,8 @@ static void parse_feature(void)

3269 die("This version of fast-import does not support feature %s.", feature);

3270 }

3271

3292 -static void parse_option(void)

3272 +static void parse_option(const char *option)

3273 {

3294 - char *option = command_buf.buf + 11;

3295 -

3274 if (seen_data_command)

3275 die("Got option command '%s' after data command", option);

3276

@@ -3408,26 +3386,27 @@ int main(int argc, char **argv)

3386 set_die_routine(die_nicely);

3387 set_checkpoint_signal();

3388 while (read_next_command() != EOF) {

3389 + const char *v;

3390 if (!strcmp("blob", command_buf.buf))

3391 parse_new_blob();

3413 - else if (starts_with(command_buf.buf, "ls "))

3414 - parse_ls(NULL);

3415 - else if (starts_with(command_buf.buf, "commit "))

3416 - parse_new_commit();

3417 - else if (starts_with(command_buf.buf, "tag "))

3418 - parse_new_tag();

3419 - else if (starts_with(command_buf.buf, "reset "))

3420 - parse_reset_branch();

3392 + else if (skip_prefix(command_buf.buf, "ls ", &v))

3393 + parse_ls(v, NULL);

3394 + else if (skip_prefix(command_buf.buf, "commit ", &v))

3395 + parse_new_commit(v);

3396 + else if (skip_prefix(command_buf.buf, "tag ", &v))

3397 + parse_new_tag(v);

3398 + else if (skip_prefix(command_buf.buf, "reset ", &v))

3399 + parse_reset_branch(v);

3400 else if (!strcmp("checkpoint", command_buf.buf))

3401 parse_checkpoint();

3402 else if (!strcmp("done", command_buf.buf))

3403 break;

3404 else if (starts_with(command_buf.buf, "progress "))

3405 parse_progress();

3427 - else if (starts_with(command_buf.buf, "feature "))

3428 - parse_feature();

3429 - else if (starts_with(command_buf.buf, "option git "))

3430 - parse_option();

3406 + else if (skip_prefix(command_buf.buf, "feature ", &v))

3407 + parse_feature(v);

3408 + else if (skip_prefix(command_buf.buf, "option git ", &v))

3409 + parse_option(v);

3410 else if (starts_with(command_buf.buf, "option "))

3411 /* ignore non-git options*/;

3412 else