upload-pack: make check_non_tip() 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@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Nguyễn Thái Ngọc Duy committed Jun 12, 2016 at 17:53 UTC 7fcbd37f9c1b7413b408d0223344d070613777ac
1 file changed +16 -7
upload-pack.c
+16 -7
@@ -475,16 +475,16 @@ static void check_non_tip(void)
475 cmd.in = -1;
476 cmd.out = -1;
477
478 - if (start_command(&cmd))
479 - goto error;
480 -
478 /*
482 - * If rev-list --stdin encounters an unknown commit, it
483 - * terminates, which will cause SIGPIPE in the write loop
479 + * If the next rev-list --stdin encounters an unknown commit,
480 + * it terminates, which will cause SIGPIPE in the write loop
481 * below.
482 */
483 sigchain_push(SIGPIPE, SIG_IGN);
484
485 + if (start_command(&cmd))
486 + goto error;
487 +
488 namebuf[0] = '^';
489 namebuf[41] = '\n';
490 for (i = get_max_object_index(); 0 < i; ) {
@@ -507,8 +507,7 @@ static void check_non_tip(void)
507 goto error;
508 }
509 close(cmd.in);
510 -
511 - sigchain_pop(SIGPIPE);
510 + cmd.in = -1;
511
512 /*
513 * The commits out of the rev-list are not ancestors of
@@ -518,6 +517,7 @@ static void check_non_tip(void)
517 if (i)
518 goto error;
519 close(cmd.out);
520 + cmd.out = -1;
521
522 /*
523 * rev-list may have died by encountering a bad commit
@@ -527,10 +527,19 @@ static void check_non_tip(void)
527 if (finish_command(&cmd))
528 goto error;
529
530 + sigchain_pop(SIGPIPE);
531 +
532 /* All the non-tip ones are ancestors of what we advertised */
533 return;
534
535 error:
536 + sigchain_pop(SIGPIPE);
537 +
538 + if (cmd.in >= 0)
539 + close(cmd.in);
540 + if (cmd.out >= 0)
541 + close(cmd.out);
542 +
543 /* Pick one of them (we know there at least is one) */
544 for (i = 0; i < want_obj.nr; i++) {
545 o = want_obj.objects[i].item;