{"thread":{"id":"60189","subject":"[PATCH] sequencer: remove unreachable exit condition in pick_commits()","startedAt":"2023-09-03T15:11:38Z","lastAt":"2023-09-13T00:32:55Z","messageCount":5,"participants":["Oswald Buddenhagen","Phillip Wood","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"481352","messageId":"20230903151132.739151-1-oswald.buddenhagen@gmx.de","threadId":"60189","inReplyTo":null,"subject":"[PATCH] sequencer: remove unreachable exit condition in pick_commits()","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-09-03T15:11:32Z","receivedAt":"2023-09-03T15:11:38Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"This was introduced by 56dc3ab04 (\"sequencer (rebase -i): implement the\n'edit' command\", 2017-01-02), and was pointless from the get-go, as the\ncommand causes an early return.\n\nSigned-off-by: Oswald Buddenhagen \n\n---\nthis remains valid after phillip's pending series, through it becomes\nmarginally harder to prove (c.f. \"sequencer: factor out part of\npick_commits()\").\n\nCc: Johannes Schindelin \nCc: Phillip Wood \n---\n sequencer.c | 4 ----\n 1 file changed, 4 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex a66dcf8ab2..99e9c520ca 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -4832,10 +4832,6 @@ static int pick_commits(struct repository *r,\n \t\tstruct strbuf head_ref = STRBUF_INIT, buf = STRBUF_INIT;\n \t\tstruct stat st;\n \n-\t\t/* Stopped in the middle, as planned? */\n-\t\tif (todo_list->current < todo_list->nr)\n-\t\t\treturn 0;\n-\n \t\tif (read_oneliner(&head_ref, rebase_path_head_name(), 0) &&\n \t\t\t\tstarts_with(head_ref.buf, \"refs/\")) {\n \t\t\tconst char *msg;\n-- \n2.40.0.152.g15d061e6df\n\n"},{"id":"481753","messageId":"ddf8dc95-6583-4257-b48a-0115f59950ef@gmail.com","threadId":"60189","inReplyTo":"20230903151132.739151-1-oswald.buddenhagen@gmx.de","subject":"Re: [PATCH] sequencer: remove unreachable exit condition in pick_commits()","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2023-09-12T10:18:35Z","receivedAt":"2023-09-12T10:18:44Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Oswald\n\nOn 03/09/2023 16:11, Oswald Buddenhagen wrote:\n> This was introduced by 56dc3ab04 (\"sequencer (rebase -i): implement the\n> 'edit' command\", 2017-01-02), and was pointless from the get-go, as the\n> command causes an early return.\n\nWhile I agree this code is unreachable, just because it was unused when \nit was added does not mean it is unused now. It would be helpful for the \ncommit message to explain that there are no \"break\" or \"goto\" statements \nin the loop body and therefore this code is only reachable when the loop \nterminates because todo_list->current == todo_list->nr\n\nBest Wishes\n\nPhillip\n\n> Signed-off-by: Oswald Buddenhagen \n> \n> ---\n> this remains valid after phillip's pending series, through it becomes\n> marginally harder to prove (c.f. \"sequencer: factor out part of\n> pick_commits()\").\n> \n> Cc: Johannes Schindelin \n> Cc: Phillip Wood \n> ---\n> sequencer.c | 4 ----\n> 1 file changed, 4 deletions(-)\n> \n> diff --git a/sequencer.c b/sequencer.c\n> index a66dcf8ab2..99e9c520ca 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -4832,10 +4832,6 @@ static int pick_commits(struct repository *r,\n> \t\tstruct strbuf head_ref = STRBUF_INIT, buf = STRBUF_INIT;\n> \t\tstruct stat st;\n> \n> -\t\t/* Stopped in the middle, as planned? */\n> -\t\tif (todo_list->current < todo_list->nr)\n> -\t\t\treturn 0;\n> -\n> \t\tif (read_oneliner(&head_ref, rebase_path_head_name(), 0) &&\n> \t\t\t\tstarts_with(head_ref.buf, \"refs/\")) {\n> \t\t\tconst char *msg;\n\n\n"},{"id":"481755","messageId":"20230912105541.272917-1-oswald.buddenhagen@gmx.de","threadId":"60189","inReplyTo":"ddf8dc95-6583-4257-b48a-0115f59950ef@gmail.com","subject":"[PATCH v2] sequencer: remove unreachable exit condition in pick_commits()","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-09-12T10:55:41Z","receivedAt":"2023-09-12T10:55:50Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"This was introduced by 56dc3ab04 (\"sequencer (rebase -i): implement the\n'edit' command\", 2017-01-02), and was pointless from the get-go: all\nearly exits from the loop above are returns, so todo_list->current ==\ntodo_list->nr is an invariant after the loop.\n\nSigned-off-by: Oswald Buddenhagen \n\n---\nv2:\n- improved commit message\n\nCc: Johannes Schindelin \nCc: Phillip Wood \nCc: Junio C Hamano \n---\n sequencer.c | 4 ----\n 1 file changed, 4 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex a66dcf8ab2..99e9c520ca 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -4832,10 +4832,6 @@ static int pick_commits(struct repository *r,\n \t\tstruct strbuf head_ref = STRBUF_INIT, buf = STRBUF_INIT;\n \t\tstruct stat st;\n \n-\t\t/* Stopped in the middle, as planned? */\n-\t\tif (todo_list->current < todo_list->nr)\n-\t\t\treturn 0;\n-\n \t\tif (read_oneliner(&head_ref, rebase_path_head_name(), 0) &&\n \t\t\t\tstarts_with(head_ref.buf, \"refs/\")) {\n \t\t\tconst char *msg;\n-- \n2.42.0.419.g70bf8a5751\n\n"},{"id":"481762","messageId":"7ede7c26-9029-4e4b-81a3-f992eff74124@gmail.com","threadId":"60189","inReplyTo":"20230912105541.272917-1-oswald.buddenhagen@gmx.de","subject":"Re: [PATCH v2] sequencer: remove unreachable exit condition in pick_commits()","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2023-09-12T13:27:34Z","receivedAt":"2023-09-12T13:27:39Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 12/09/2023 11:55, Oswald Buddenhagen wrote:\n> This was introduced by 56dc3ab04 (\"sequencer (rebase -i): implement the\n> 'edit' command\", 2017-01-02), and was pointless from the get-go: all\n> early exits from the loop above are returns, so todo_list->current ==\n> todo_list->nr is an invariant after the loop.\n> \n> Signed-off-by: Oswald Buddenhagen \n\nThanks for updating the commit message, I think it is clearer now\n\nBest Wishes\n\nPhillip\n\n> ---\n> v2:\n> - improved commit message\n> \n> Cc: Johannes Schindelin \n> Cc: Phillip Wood \n> Cc: Junio C Hamano \n> ---\n> sequencer.c | 4 ----\n> 1 file changed, 4 deletions(-)\n> \n> diff --git a/sequencer.c b/sequencer.c\n> index a66dcf8ab2..99e9c520ca 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -4832,10 +4832,6 @@ static int pick_commits(struct repository *r,\n> \t\tstruct strbuf head_ref = STRBUF_INIT, buf = STRBUF_INIT;\n> \t\tstruct stat st;\n> \n> -\t\t/* Stopped in the middle, as planned? */\n> -\t\tif (todo_list->current < todo_list->nr)\n> -\t\t\treturn 0;\n> -\n> \t\tif (read_oneliner(&head_ref, rebase_path_head_name(), 0) &&\n> \t\t\t\tstarts_with(head_ref.buf, \"refs/\")) {\n> \t\t\tconst char *msg;\n\n"},{"id":"481797","messageId":"xmqqwmwukj4f.fsf@gitster.g","threadId":"60189","inReplyTo":"7ede7c26-9029-4e4b-81a3-f992eff74124@gmail.com","subject":"Re: [PATCH v2] sequencer: remove unreachable exit condition in pick_commits()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-09-13T00:32:48Z","receivedAt":"2023-09-13T00:32:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> On 12/09/2023 11:55, Oswald Buddenhagen wrote:\n>> This was introduced by 56dc3ab04 (\"sequencer (rebase -i): implement the\n>> 'edit' command\", 2017-01-02), and was pointless from the get-go: all\n>> early exits from the loop above are returns, so todo_list->current ==\n>> todo_list->nr is an invariant after the loop.\n>> Signed-off-by: Oswald Buddenhagen \n>\n> Thanks for updating the commit message, I think it is clearer now\n\nThanks, both. Queued.\n"}]}