From: Ramkumar Ramachandra Date: Thu, 07 Jul 2011 06:31:27 GMT Subject: Re: [PATCH 11/14] revert: Introduce a layer of indirection over pick_commits Message-ID: In-Reply-To: <7vd3hnktuf.fsf@alter.siamese.dyndns.org> Hi Junio, Junio C Hamano writes: > Ramkumar Ramachandra writes: >> diff --git a/builtin/revert.c b/builtin/revert.c >> index 7d76f92..8cdcdb6 100644 >> --- a/builtin/revert.c >> +++ b/builtin/revert.c >> @@ -677,10 +677,8 @@ static int cleanup_sequencer_data(void) >>       return 0; >>  } >> >> -static int pick_commits(struct replay_opts *opts) >> +static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts) >>  { >> -     struct commit_list *todo_list = NULL; >> -     unsigned char sha1[20]; >>       struct commit_list *cur; >>       int res; >> >> @@ -690,12 +688,6 @@ static int pick_commits(struct replay_opts *opts) >>                               opts->record_origin || opts->edit)); >>       read_and_refresh_cache(me, opts); >> >> -     walk_revs_populate_todo(&todo_list, opts); >> -     create_seq_dir(); >> -     if (!get_sha1("HEAD", sha1)) >> -             save_head(sha1_to_hex(sha1)); >> -     save_todo(todo_list, opts); >> - >>       for (cur = todo_list; cur; cur = cur->next) { >>               save_todo(cur, opts); >>               res = do_pick_commit(cur->item, opts); >> @@ -710,6 +702,22 @@ static int pick_commits(struct replay_opts *opts) >>       return cleanup_sequencer_data(); >>  } >> >> +static int process_continuation(struct replay_opts *opts) >> +{ >> +     struct commit_list *todo_list = NULL; >> +     unsigned char sha1[20]; >> + >> +     read_and_refresh_cache(me, opts); >> + >> +     walk_revs_populate_todo(&todo_list, opts); >> +     create_seq_dir(); >> +     if (!get_sha1("HEAD", sha1)) >> +             persist_head(sha1_to_hex(sha1)); >> +     persist_todo(todo_list, opts); > > Don't these two need forward declarations before their use? Good catch. I'm surprised the compiler didn't catch these. -- Ram