Nguyễn Thái Ngọc Duy <[email protected]> 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 <[email protected]> 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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
