{"thread":{"id":"58960","subject":"[PATCH] bisect: remove check for fp and move the goto label after the fclose","startedAt":"2022-12-16T00:43:28Z","lastAt":"2022-12-16T01:03:34Z","messageCount":2,"participants":["Rose via GitGitGadget","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"469150","messageId":"pull.1397.git.git.1671151402532.gitgitgadget@gmail.com","threadId":"58960","inReplyTo":null,"subject":"[PATCH] bisect: remove check for fp and move the goto label after the fclose","fromName":"Rose via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-12-16T00:43:22Z","receivedAt":"2022-12-16T00:43:28Z","isPatch":true,"sender":{"key":"ckelsch@jgrcpa.com","avatar":null},"body":"From: Seija Kijin <doremylover123@gmail.com>\n\nThe fp check in these function are only relevant if goto has been taken.\n\nBecause of this, we should move the label so that fclose is not called.\n\nThis is because the goto is taken when fp is 0 anyway.\n\nSigned-off-by: Seija Kijin <doremylover123@gmail.com>\n---\n    bisect: remove check for fp and move the goto label after the fclose\n    \n    The fp check in these function are only relevant if goto has been taken.\n    \n    Because of this, we should move the label so that fclose is not called.\n    \n    This is because the goto is taken when fp is 0 anyway.\n    \n    Signed-off-by: Seija Kijin doremylover123@gmail.com\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1397%2FAtariDreams%2Ffclose-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1397/AtariDreams/fclose-v1\nPull-Request: https://github.com/git/git/pull/1397\n\n builtin/bisect.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/bisect.c b/builtin/bisect.c\nindex cc9483e8515..2e5dbc8be3b 100644\n--- a/builtin/bisect.c\n+++ b/builtin/bisect.c\n@@ -316,9 +316,9 @@ static int bisect_write(const char *state, const char *rev,\n \tif (!nolog)\n \t\tfprintf(fp, \"git bisect %s %s\\n\", state, rev);\n \n+\tfclose(fp);\n+\n finish:\n-\tif (fp)\n-\t\tfclose(fp);\n \tstrbuf_release(&tag);\n \treturn res;\n }\n@@ -484,9 +484,9 @@ static int get_terms(struct bisect_terms *terms)\n \tstrbuf_getline_lf(&str, fp);\n \tterms->term_good = strbuf_detach(&str, NULL);\n \n+\tfclose(fp);\n+\n finish:\n-\tif (fp)\n-\t\tfclose(fp);\n \tstrbuf_release(&str);\n \treturn res;\n }\n\nbase-commit: 57e2c6ebbe7108b35ba30184dcbcb6c34c929ad8\n-- \ngitgitgadget\n"},{"id":"469151","messageId":"xmqqy1r8nork.fsf@gitster.g","threadId":"58960","inReplyTo":"pull.1397.git.git.1671151402532.gitgitgadget@gmail.com","subject":"Re: [PATCH] bisect: remove check for fp and move the goto label after the fclose","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-12-16T01:03:27Z","receivedAt":"2022-12-16T01:03:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Rose via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: Seija Kijin <doremylover123@gmail.com>\n>\n> The fp check in these function are only relevant if goto has been taken.\n>\n> Because of this, we should move the label so that fclose is not called.\n>\n> This is because the goto is taken when fp is 0 anyway.\n\nIt may happen to be so with today's code, but for the health of the\ncode in the longer term, I do not think this patch is a good idea.\nWe may add more \"check error and jump to the label to clean it up\"\nexit paths to these functions, and change to add such exit paths\nwould essentially need to revert this change.\n\nBy the way, I do not know if this is something GGG end-users can\nfix, or requires a fix in GGG itself, but somebody keeps adding an\ne-mail address in the @users.noreply.github.com domain to the Cc:\nfield of these patches.  For folks whose work is GitHub only, it\nmight be fine to declare \"this user only has e-mail-address-looking\nstring but does not receive e-mail at all\", but as working with GGG\nis all about following e-mail based workflow, such an address should\nbe added to patch e-mails sent to the list this say.\n\nThanks.\n"}]}