git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Rose via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Seija Kijin <doremylover123@gmail.com>
Subject: Re: [PATCH] bisect: remove check for fp and move the goto label after the fclose
Date: Fri, 16 Dec 2022 10:03:27 +0900	[thread overview]
Message-ID: <xmqqy1r8nork.fsf@gitster.g> (raw)
In-Reply-To: <pull.1397.git.git.1671151402532.gitgitgadget@gmail.com> (Rose via GitGitGadget's message of "Fri, 16 Dec 2022 00:43:22 +0000")

"Rose via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Seija Kijin <doremylover123@gmail.com>
>
> The fp check in these function are only relevant if goto has been taken.
>
> Because of this, we should move the label so that fclose is not called.
>
> This is because the goto is taken when fp is 0 anyway.

It may happen to be so with today's code, but for the health of the
code in the longer term, I do not think this patch is a good idea.
We may add more "check error and jump to the label to clean it up"
exit paths to these functions, and change to add such exit paths
would essentially need to revert this change.

By the way, I do not know if this is something GGG end-users can
fix, or requires a fix in GGG itself, but somebody keeps adding an
e-mail address in the @users.noreply.github.com domain to the Cc:
field of these patches.  For folks whose work is GitHub only, it
might be fine to declare "this user only has e-mail-address-looking
string but does not receive e-mail at all", but as working with GGG
is all about following e-mail based workflow, such an address should
be added to patch e-mails sent to the list this say.

Thanks.

      reply	other threads:[~2022-12-16  1:03 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-16  0:43 [PATCH] bisect: remove check for fp and move the goto label after the fclose Rose via GitGitGadget
2022-12-16  1:03 ` Junio C Hamano [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqy1r8nork.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=doremylover123@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).