{"thread":{"id":"54021","subject":"[PATCH] upload-pack: remove superfluous sigchain_pop() call","startedAt":"2020-08-11T17:15:18Z","lastAt":"2020-08-11T21:01:45Z","messageCount":3,"participants":["René Scharfe","Taylor Blau","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"403366","messageId":"29ac165c-8458-9bb1-7e59-e7c2c68a9f2a@web.de","threadId":"54021","inReplyTo":null,"subject":"[PATCH] upload-pack: remove superfluous sigchain_pop() call","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2020-08-11T17:15:03Z","receivedAt":"2020-08-11T17:15:18Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"2997178ee6 (upload-pack: split check_unreachable() in two, prep for\nget_reachable_list(), 2016-06-12) moved most code of has_unreachable()\ninto the new function do_reachable_revlist(). The latter takes care to\nignore SIGPIPE during its operations, and restores the original signal\nhandler before returning.\n\nHowever, a sigchain_pop(SIGPIPE) call remained in the error handling\ncode of has_unreachable(), which does nothing because the stack is\nempty after do_reachable_revlist() cleaned up after itself. Remove it.\n\nSigned-off-by: René Scharfe \n---\n upload-pack.c | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/upload-pack.c b/upload-pack.c\nindex d087113d23e..1b068da0d26 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -731,7 +731,6 @@ static int has_unreachable(struct object_array *src, enum allow_uor allow_uor)\n \treturn 0;\n\n error:\n-\tsigchain_pop(SIGPIPE);\n \tif (cmd.out >= 0)\n \t\tclose(cmd.out);\n \treturn 1;\n--\n2.28.0\n"},{"id":"403370","messageId":"20200811182603.GB33504@syl.lan","threadId":"54021","inReplyTo":"29ac165c-8458-9bb1-7e59-e7c2c68a9f2a@web.de","subject":"Re: [PATCH] upload-pack: remove superfluous sigchain_pop() call","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2020-08-11T18:26:03Z","receivedAt":"2020-08-11T18:26:08Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Aug 11, 2020 at 07:15:03PM +0200, René Scharfe wrote:\n> 2997178ee6 (upload-pack: split check_unreachable() in two, prep for\n> get_reachable_list(), 2016-06-12) moved most code of has_unreachable()\n> into the new function do_reachable_revlist(). The latter takes care to\n> ignore SIGPIPE during its operations, and restores the original signal\n> handler before returning.\n>\n> However, a sigchain_pop(SIGPIPE) call remained in the error handling\n> code of has_unreachable(), which does nothing because the stack is\n> empty after do_reachable_revlist() cleaned up after itself. Remove it.\n\nThanks for noticing. Your analysis seems correct to me after a quick\ninspection of the code, so this has my:\n\n Reviewed-by: Taylor Blau \n\n> Signed-off-by: René Scharfe \n> ---\n> upload-pack.c | 1 -\n> 1 file changed, 1 deletion(-)\n>\n> diff --git a/upload-pack.c b/upload-pack.c\n> index d087113d23e..1b068da0d26 100644\n> --- a/upload-pack.c\n> +++ b/upload-pack.c\n> @@ -731,7 +731,6 @@ static int has_unreachable(struct object_array *src, enum allow_uor allow_uor)\n> \treturn 0;\n>\n> error:\n> -\tsigchain_pop(SIGPIPE);\n> \tif (cmd.out >= 0)\n> \t\tclose(cmd.out);\n> \treturn 1;\n> --\n> 2.28.0\n\nThanks,\nTaylor\n"},{"id":"403404","messageId":"xmqqtux899sc.fsf@gitster.c.googlers.com","threadId":"54021","inReplyTo":"20200811182603.GB33504@syl.lan","subject":"Re: [PATCH] upload-pack: remove superfluous sigchain_pop() call","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-11T21:01:39Z","receivedAt":"2020-08-11T21:01:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau writes:\n\n> On Tue, Aug 11, 2020 at 07:15:03PM +0200, René Scharfe wrote:\n>> 2997178ee6 (upload-pack: split check_unreachable() in two, prep for\n>> get_reachable_list(), 2016-06-12) moved most code of has_unreachable()\n>> into the new function do_reachable_revlist(). The latter takes care to\n>> ignore SIGPIPE during its operations, and restores the original signal\n>> handler before returning.\n>>\n>> However, a sigchain_pop(SIGPIPE) call remained in the error handling\n>> code of has_unreachable(), which does nothing because the stack is\n>> empty after do_reachable_revlist() cleaned up after itself. Remove it.\n>\n> Thanks for noticing. Your analysis seems correct to me after a quick\n> inspection of the code, so this has my:\n>\n> Reviewed-by: Taylor Blau \n>\n>> Signed-off-by: René Scharfe \n>> ---\n>> upload-pack.c | 1 -\n>> 1 file changed, 1 deletion(-)\n\nYeah, looks good to me, too. Thanks, both.\n\n\n>> diff --git a/upload-pack.c b/upload-pack.c\n>> index d087113d23e..1b068da0d26 100644\n>> --- a/upload-pack.c\n>> +++ b/upload-pack.c\n>> @@ -731,7 +731,6 @@ static int has_unreachable(struct object_array *src, enum allow_uor allow_uor)\n>> \treturn 0;\n>>\n>> error:\n>> -\tsigchain_pop(SIGPIPE);\n>> \tif (cmd.out >= 0)\n>> \t\tclose(cmd.out);\n>> \treturn 1;\n>> --\n>> 2.28.0\n>\n> Thanks,\n> Taylor\n"}]}