Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> writes:
> Subject: Re: [PATCH 09/27] upload-pack: make check_non_tip() clean things up error
"clean things up on error"?
> On error check_non_tip() will die and not closing file descriptors is no
> big deal. The next patch will split the majority of this function out
> for reuse in other cases, where die() may not be the only outcome. Same
> story for popping SIGPIPE out of the signal chain. So let's make sure we
> clean things up properly first.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
Makes me wonder if you can push into sigchain before the first
appearance of "goto error" so that in the error handling codepath
you can do sigchain_pop(), without adding sigchain_pop() before all
the "goto error"?
> upload-pack.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/upload-pack.c b/upload-pack.c
> index 60f2e5e..1e8b025 100644
> --- a/upload-pack.c
> +++ b/upload-pack.c
> @@ -494,8 +494,10 @@ static void check_non_tip(void)
> if (!is_our_ref(o))
> continue;
> memcpy(namebuf + 1, oid_to_hex(&o->oid), GIT_SHA1_HEXSZ);
> - if (write_in_full(cmd.in, namebuf, 42) < 0)
> + if (write_in_full(cmd.in, namebuf, 42) < 0) {
> + sigchain_pop(SIGPIPE);
> goto error;
> + }
> }
> namebuf[40] = '\n';
> for (i = 0; i < want_obj.nr; i++) {
> @@ -503,10 +505,13 @@ static void check_non_tip(void)
> if (is_our_ref(o))
> continue;
> memcpy(namebuf, oid_to_hex(&o->oid), GIT_SHA1_HEXSZ);
> - if (write_in_full(cmd.in, namebuf, 41) < 0)
> + if (write_in_full(cmd.in, namebuf, 41) < 0) {
> + sigchain_pop(SIGPIPE);
> goto error;
> + }
> }
> close(cmd.in);
> + cmd.in = -1;
>
> sigchain_pop(SIGPIPE);
>
> @@ -518,6 +523,7 @@ static void check_non_tip(void)
> if (i)
> goto error;
> close(cmd.out);
> + cmd.out = -1;
>
> /*
> * rev-list may have died by encountering a bad commit
> @@ -531,6 +537,11 @@ static void check_non_tip(void)
> return;
>
> error:
> + if (cmd.in >= 0)
> + close(cmd.in);
> + if (cmd.out >= 0)
> + close(cmd.out);
> +
> /* Pick one of them (we know there at least is one) */
> for (i = 0; i < want_obj.nr; i++) {
> o = want_obj.objects[i].item;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html