{"thread":{"id":"49895","subject":"[PATCH] t/lib-git-daemon: fix signal checking","startedAt":"2018-11-26T20:03:45Z","lastAt":"2018-12-04T06:34:06Z","messageCount":2,"participants":["SZEDER Gábor","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"364097","messageId":"20181126200337.32462-1-szeder.dev@gmail.com","threadId":"49895","inReplyTo":null,"subject":"[PATCH] t/lib-git-daemon: fix signal checking","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-11-26T20:03:37Z","receivedAt":"2018-11-26T20:03:45Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Test scripts checking 'git daemon' stop the daemon with a TERM signal,\nand the 'stop_git_daemon' helper checks the daemon's exit status to\nmake sure that it indeed died because of that signal.\n\nThis check is bogus since 03c39b3458 (t/lib-git-daemon: use\ntest_match_signal, 2016-06-24), for two reasons:\n\n  - Right after killing 'git daemon', 'stop_git_daemon' saves its exit\n    status in a variable, but since 03c39b3458 the condition checking\n    the exit status looks at '$?', which at this point is not the exit\n    status of 'git daemon', but that of the variable assignment, i.e.\n    it's always 0.\n\n  - The unexpected exit status should abort the whole test script with\n    'error', but it doesn't, because 03c39b3458 forgot to negate\n    'test_match_signal's exit status in the condition.\n\nThis patch fixes both issues.\n\nSigned-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n---\n t/lib-git-daemon.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh\nindex edbea2d986..f98de95c15 100644\n--- a/t/lib-git-daemon.sh\n+++ b/t/lib-git-daemon.sh\n@@ -92,7 +92,7 @@ stop_git_daemon() {\n \tkill \"$GIT_DAEMON_PID\"\n \twait \"$GIT_DAEMON_PID\" >&3 2>&4\n \tret=$?\n-\tif test_match_signal 15 $?\n+\tif ! test_match_signal 15 $ret\n \tthen\n \t\terror \"git daemon exited with status: $ret\"\n \tfi\n-- \n2.20.0.rc1.149.g55c2c038c2\n\n"},{"id":"364537","messageId":"20181204063402.GA10178@sigill.intra.peff.net","threadId":"49895","inReplyTo":"20181126200337.32462-1-szeder.dev@gmail.com","subject":"Re: [PATCH] t/lib-git-daemon: fix signal checking","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-12-04T06:34:03Z","receivedAt":"2018-12-04T06:34:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 26, 2018 at 09:03:37PM +0100, SZEDER Gábor wrote:\n\n> Test scripts checking 'git daemon' stop the daemon with a TERM signal,\n> and the 'stop_git_daemon' helper checks the daemon's exit status to\n> make sure that it indeed died because of that signal.\n> \n> This check is bogus since 03c39b3458 (t/lib-git-daemon: use\n> test_match_signal, 2016-06-24), for two reasons:\n> \n>   - Right after killing 'git daemon', 'stop_git_daemon' saves its exit\n>     status in a variable, but since 03c39b3458 the condition checking\n>     the exit status looks at '$?', which at this point is not the exit\n>     status of 'git daemon', but that of the variable assignment, i.e.\n>     it's always 0.\n> \n>   - The unexpected exit status should abort the whole test script with\n>     'error', but it doesn't, because 03c39b3458 forgot to negate\n>     'test_match_signal's exit status in the condition.\n> \n> This patch fixes both issues.\n\nOof. Who says two wrongs don't make a right? :)\n\nThanks for catching this, and the patch looks obviously correct.\n\nI peeked at the other test_match_signal conversions from that era, and\nthey all look sane.\n\n-Peff\n"}]}