Re: [PATCH v2 2/3] sequencer: make commit options more extensible
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 24, 2017, 01:01 UTC
- Message-ID
- <xmqq7f3fa3oz.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <75e2c4fe839602af19811ed3e251eb583ff6c8ad.1490285210.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
Show 23 quoted lines
> @@ -926,14 +930,14 @@ static void record_in_rewritten(struct object_id *oid,
> static int do_pick_commit(enum todo_command command, struct commit *commit,
> struct replay_opts *opts, int final_fixup)
> {
> - int edit = opts->edit, cleanup_commit_message = 0;
> - const char *msg_file = edit ? NULL : git_path_merge_msg();
> + unsigned int flags = opts->edit ? EDIT_MSG : 0, allow = 0;
> + const char *msg_file = opts->edit ? NULL : git_path_merge_msg();
> unsigned char head[20];
> struct commit *base, *next, *parent;
> const char *base_label, *next_label;
> struct commit_message msg = { NULL, NULL, NULL, NULL };
> struct strbuf msgbuf = STRBUF_INIT;
> - int res, unborn = 0, amend = 0, allow = 0;
> + int res, unborn = 0;
> ...
> @@ -1123,11 +1127,11 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
> if (allow < 0) {
> res = allow;
> goto leave;
> - }
> + } else if (allow)
> + flags |= ALLOW_EMPTY;Making "flags" unsigned was a correct change, but this is now wrong, as "allow" is made unsigned by accident. It still needs to receive the return value from allow_empty() that can signal an error by returning a negative value. It seems you did this deliberately (it wasn't like "flags" and "allow" were on the same line both "int" and the change to make "flags" unsigned accidentally made "allow" also unsigned; "allow" was "int" and didn't have to move when "flags" was introduced in the first place), but I failed to spot it because I was only looking at s/int/unsigned/ on the line in the interdiff, and missed that "allow" was on the same line. Compilers are better at finding these bugs.
Perhaps something like this needs to be squashed in?
sequencer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sequencer.c b/sequencer.c index bc2fe48e65..6c423566e6 100644 --- a/sequencer.c +++ b/sequencer.c @@ -932,14 +932,14 @@ static void record_in_rewritten(struct object_id *oid, static int do_pick_commit(enum todo_command command, struct commit *commit, struct replay_opts *opts, int final_fixup) { - unsigned int flags = opts->edit ? EDIT_MSG : 0, allow = 0; + unsigned int flags = opts->edit ? EDIT_MSG : 0; const char *msg_file = opts->edit ? NULL : git_path_merge_msg(); unsigned char head[20]; struct commit *base, *next, *parent; const char *base_label, *next_label; struct commit_message msg = { NULL, NULL, NULL, NULL }; struct strbuf msgbuf = STRBUF_INIT; - int res, unborn = 0; + int allow, res, unborn = 0; if (opts->no_commit) { /*
-- 2.12.1-432-gf364f02724