Skip to content

Commit 97313be

Browse files
peffgitster
peff
authored and
gitster
committed
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>
1 parent 95b567c commit 97313be

File tree

1 file changed

+51
-72
lines changed

1 file changed

+51
-72
lines changed

‎fast-import.c‎

Lines changed: 51 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -371,8 +371,8 @@ static volatile sig_atomic_t checkpoint_requested;
371371
static int cat_blob_fd = STDOUT_FILENO;
372372

373373
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);
376376

377377
static void write_branch_report(FILE *rpt, struct branch *b)
378378
{
@@ -1861,6 +1861,8 @@ static int read_next_command(void)
18611861
}
18621862

18631863
for (;;) {
1864+
const char *p;
1865+
18641866
if (unread_command_buf) {
18651867
unread_command_buf = 0;
18661868
} else {
@@ -1893,8 +1895,8 @@ static int read_next_command(void)
18931895
rc->prev->next = rc;
18941896
cmd_tail = rc;
18951897
}
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);
18981900
continue;
18991901
}
19001902
if (command_buf.buf[0] == '#')
@@ -2273,9 +2275,8 @@ static uintmax_t parse_mark_ref_space(const char **p)
22732275
return mark;
22742276
}
22752277

2276-
static void file_change_m(struct branch *b)
2278+
static void file_change_m(const char *p, struct branch *b)
22772279
{
2278-
const char *p = command_buf.buf + 2;
22792280
static struct strbuf uq = STRBUF_INIT;
22802281
const char *endp;
22812282
struct object_entry *oe;
@@ -2376,9 +2377,8 @@ static void file_change_m(struct branch *b)
23762377
tree_content_set(&b->branch_tree, p, sha1, mode, NULL);
23772378
}
23782379

2379-
static void file_change_d(struct branch *b)
2380+
static void file_change_d(const char *p, struct branch *b)
23802381
{
2381-
const char *p = command_buf.buf + 2;
23822382
static struct strbuf uq = STRBUF_INIT;
23832383
const char *endp;
23842384

@@ -2391,15 +2391,14 @@ static void file_change_d(struct branch *b)
23912391
tree_content_remove(&b->branch_tree, p, NULL, 1);
23922392
}
23932393

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)
23952395
{
2396-
const char *s, *d;
2396+
const char *d;
23972397
static struct strbuf s_uq = STRBUF_INIT;
23982398
static struct strbuf d_uq = STRBUF_INIT;
23992399
const char *endp;
24002400
struct tree_entry leaf;
24012401

2402-
s = command_buf.buf + 2;
24032402
strbuf_reset(&s_uq);
24042403
if (!unquote_c_style(&s_uq, s, &endp)) {
24052404
if (*endp != ' ')
@@ -2444,9 +2443,8 @@ static void file_change_cr(struct branch *b, int rename)
24442443
leaf.tree);
24452444
}
24462445

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)
24482447
{
2449-
const char *p = command_buf.buf + 2;
24502448
static struct strbuf uq = STRBUF_INIT;
24512449
struct object_entry *oe;
24522450
struct branch *s;
@@ -2587,15 +2585,14 @@ static int parse_from(struct branch *b)
25872585
const char *from;
25882586
struct branch *s;
25892587

2590-
if (!starts_with(command_buf.buf, "from "))
2588+
if (!skip_prefix(command_buf.buf, "from ", &from))
25912589
return 0;
25922590

25932591
if (b->branch_tree.tree) {
25942592
release_tree_content_recursive(b->branch_tree.tree);
25952593
b->branch_tree.tree = NULL;
25962594
}
25972595

2598-
from = strchr(command_buf.buf, ' ') + 1;
25992596
s = lookup_branch(from);
26002597
if (b == s)
26012598
die("Can't create a branch from itself: %s", b->name);
@@ -2636,8 +2633,7 @@ static struct hash_list *parse_merge(unsigned int *count)
26362633
struct branch *s;
26372634

26382635
*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)) {
26412637
n = xmalloc(sizeof(*n));
26422638
s = lookup_branch(from);
26432639
if (s)
@@ -2668,23 +2664,20 @@ static struct hash_list *parse_merge(unsigned int *count)
26682664
return list;
26692665
}
26702666

2671-
static void parse_new_commit(void)
2667+
static void parse_new_commit(const char *arg)
26722668
{
26732669
static struct strbuf msg = STRBUF_INIT;
26742670
struct branch *b;
2675-
char *sp;
26762671
char *author = NULL;
26772672
char *committer = NULL;
26782673
struct hash_list *merge_list = NULL;
26792674
unsigned int merge_count;
26802675
unsigned char prev_fanout, new_fanout;
26812676
const char *v;
26822677

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);
26862679
if (!b)
2687-
b = new_branch(sp);
2680+
b = new_branch(arg);
26882681

26892682
read_next_command();
26902683
parse_mark();
@@ -2713,20 +2706,20 @@ static void parse_new_commit(void)
27132706

27142707
/* file_change* */
27152708
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);
27262719
else if (!strcmp("deleteall", command_buf.buf))
27272720
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);
27302723
else {
27312724
unread_command_buf = 1;
27322725
break;
@@ -2769,10 +2762,9 @@ static void parse_new_commit(void)
27692762
b->last_commit = object_count_by_type[OBJ_COMMIT];
27702763
}
27712764

2772-
static void parse_new_tag(void)
2765+
static void parse_new_tag(const char *arg)
27732766
{
27742767
static struct strbuf msg = STRBUF_INIT;
2775-
char *sp;
27762768
const char *from;
27772769
char *tagger;
27782770
struct branch *s;
@@ -2782,11 +2774,9 @@ static void parse_new_tag(void)
27822774
enum object_type type;
27832775
const char *v;
27842776

2785-
/* Obtain the new tag name from the rest of our command */
2786-
sp = strchr(command_buf.buf, ' ') + 1;
27872777
t = pool_alloc(sizeof(struct tag));
27882778
memset(t, 0, sizeof(struct tag));
2789-
t->name = pool_strdup(sp);
2779+
t->name = pool_strdup(arg);
27902780
if (last_tag)
27912781
last_tag->next_tag = t;
27922782
else
@@ -2795,9 +2785,8 @@ static void parse_new_tag(void)
27952785
read_next_command();
27962786

27972787
/* from ... */
2798-
if (!starts_with(command_buf.buf, "from "))
2788+
if (!skip_prefix(command_buf.buf, "from ", &from))
27992789
die("Expected from command, got %s", command_buf.buf);
2800-
from = strchr(command_buf.buf, ' ') + 1;
28012790
s = lookup_branch(from);
28022791
if (s) {
28032792
if (is_null_sha1(s->sha1))
@@ -2853,14 +2842,11 @@ static void parse_new_tag(void)
28532842
t->pack_id = pack_id;
28542843
}
28552844

2856-
static void parse_reset_branch(void)
2845+
static void parse_reset_branch(const char *arg)
28572846
{
28582847
struct branch *b;
2859-
char *sp;
28602848

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);
28642850
if (b) {
28652851
hashclr(b->sha1);
28662852
hashclr(b->branch_tree.versions[0].sha1);
@@ -2871,7 +2857,7 @@ static void parse_reset_branch(void)
28712857
}
28722858
}
28732859
else
2874-
b = new_branch(sp);
2860+
b = new_branch(arg);
28752861
read_next_command();
28762862
parse_from(b);
28772863
if (command_buf.len > 0)
@@ -2929,14 +2915,12 @@ static void cat_blob(struct object_entry *oe, unsigned char sha1[20])
29292915
free(buf);
29302916
}
29312917

2932-
static void parse_cat_blob(void)
2918+
static void parse_cat_blob(const char *p)
29332919
{
2934-
const char *p;
29352920
struct object_entry *oe = oe;
29362921
unsigned char sha1[20];
29372922

29382923
/* cat-blob SP <object> LF */
2939-
p = command_buf.buf + strlen("cat-blob ");
29402924
if (*p == ':') {
29412925
oe = find_mark(parse_mark_ref_eol(p));
29422926
if (!oe)
@@ -3053,14 +3037,12 @@ static void print_ls(int mode, const unsigned char *sha1, const char *path)
30533037
cat_blob_write(line.buf, line.len);
30543038
}
30553039

3056-
static void parse_ls(struct branch *b)
3040+
static void parse_ls(const char *p, struct branch *b)
30573041
{
3058-
const char *p;
30593042
struct tree_entry *root = NULL;
30603043
struct tree_entry leaf = {NULL};
30613044

30623045
/* ls SP (<tree-ish> SP)? <path> */
3063-
p = command_buf.buf + strlen("ls ");
30643046
if (*p == '"') {
30653047
if (!b)
30663048
die("Not in a commit: %s", command_buf.buf);
@@ -3276,10 +3258,8 @@ static int parse_one_feature(const char *feature, int from_stream)
32763258
return 1;
32773259
}
32783260

3279-
static void parse_feature(void)
3261+
static void parse_feature(const char *feature)
32803262
{
3281-
char *feature = command_buf.buf + 8;
3282-
32833263
if (seen_data_command)
32843264
die("Got feature command '%s' after data command", feature);
32853265

@@ -3289,10 +3269,8 @@ static void parse_feature(void)
32893269
die("This version of fast-import does not support feature %s.", feature);
32903270
}
32913271

3292-
static void parse_option(void)
3272+
static void parse_option(const char *option)
32933273
{
3294-
char *option = command_buf.buf + 11;
3295-
32963274
if (seen_data_command)
32973275
die("Got option command '%s' after data command", option);
32983276

@@ -3408,26 +3386,27 @@ int main(int argc, char **argv)
34083386
set_die_routine(die_nicely);
34093387
set_checkpoint_signal();
34103388
while (read_next_command() != EOF) {
3389+
const char *v;
34113390
if (!strcmp("blob", command_buf.buf))
34123391
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);
34213400
else if (!strcmp("checkpoint", command_buf.buf))
34223401
parse_checkpoint();
34233402
else if (!strcmp("done", command_buf.buf))
34243403
break;
34253404
else if (starts_with(command_buf.buf, "progress "))
34263405
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);
34313410
else if (starts_with(command_buf.buf, "option "))
34323411
/* ignore non-git options*/;
34333412
else

0 commit comments

Comments
 (0)