{"thread":{"id":"48218","subject":"[PATCH] Fix condition for redirecting stderr","startedAt":"2018-04-03T22:22:42Z","lastAt":"2018-04-09T20:09:35Z","messageCount":3,"participants":["Lucas Werkmeister","Todd Zullinger"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"343773","messageId":"20180403221307.29194-1-mail@lucaswerkmeister.de","threadId":"48218","inReplyTo":null,"subject":"[PATCH] Fix condition for redirecting stderr","fromName":"Lucas Werkmeister","fromEmail":"mail@lucaswerkmeister.de","sentAt":"2018-04-03T22:13:07Z","receivedAt":"2018-04-03T22:22:42Z","isPatch":true,"sender":{"key":"mail@lucaswerkmeister.de","avatar":"https://avatars.githubusercontent.com/u/2346599?v=4"},"body":"Since the --log-destination option was added in 0c591cacb (\"daemon: add\n--log-destination=(stderr|syslog|none)\", 2018-02-04) with the explicit\ngoal of allowing logging to stderr when running in inetd mode, we should\nnot always redirect stderr to /dev/null in inetd mode, but rather only\nwhen stderr is not being used for logging.\n\nSigned-off-by: Lucas Werkmeister <mail@lucaswerkmeister.de>\n---\n\nNotes:\n    I have to apologize to the list here… even though I proposed this\n    option with the goal of using it on my server, for some reason I\n    decided to only use it there after the next Git release had come\n    out, and didn’t test it anywhere else. The code looked okay, but I\n    missed this part near the end of daemon.c that made it ineffective,\n    rendering the feature broken in the 2.17.0 release and making me\n    look like an idiot :/ sorry, everyone.\n    \n    For what it’s worth, with this fix the feature appears to work as\n    intended (I *have* tested it on my server now and log messages from\n    git-daemon show up in the journal as intended, attributed to the\n    correct unit and everything).\n\n daemon.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/daemon.c b/daemon.c\nindex fe833ea7d..9d2e0d20e 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -1459,7 +1459,7 @@ int cmd_main(int argc, const char **argv)\n \t\tdie(\"base-path '%s' does not exist or is not a directory\",\n \t\t    base_path);\n \n-\tif (inetd_mode) {\n+\tif (log_destination != LOG_DESTINATION_STDERR) {\n \t\tif (!freopen(\"/dev/null\", \"w\", stderr))\n \t\t\tdie_errno(\"failed to redirect stderr to /dev/null\");\n \t}\n-- \n2.16.3\n\n"},{"id":"344147","messageId":"20180409022303.GS29680@zaya.teonanacatl.net","threadId":"48218","inReplyTo":"20180403221307.29194-1-mail@lucaswerkmeister.de","subject":"Re: [PATCH] Fix condition for redirecting stderr","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2018-04-09T02:23:04Z","receivedAt":"2018-04-09T02:23:15Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Lucas Werkmeister wrote:\n> Since the --log-destination option was added in 0c591cacb (\"daemon: add\n> --log-destination=(stderr|syslog|none)\", 2018-02-04) with the explicit\n> goal of allowing logging to stderr when running in inetd mode, we should\n> not always redirect stderr to /dev/null in inetd mode, but rather only\n> when stderr is not being used for logging.\n\nPerhaps 's/^F/daemon: f/' on the subject?  (Junio may well\nalready have done so while queueing locally.)\n\nThe patch itself looks reasonable (to my relatively untrained eyes).\n\n-- \nTodd\n~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\nHardware:  the parts of a computer that can be kicked.\n    -- Jeff Pesis\n\n"},{"id":"344232","messageId":"02f500d5-6038-a7c1-cd42-aadf33562818@lucaswerkmeister.de","threadId":"48218","inReplyTo":"20180409022303.GS29680@zaya.teonanacatl.net","subject":"Re: [PATCH] Fix condition for redirecting stderr","fromName":"Lucas Werkmeister","fromEmail":"mail@lucaswerkmeister.de","sentAt":"2018-04-09T20:09:26Z","receivedAt":"2018-04-09T20:09:35Z","isPatch":true,"sender":{"key":"mail@lucaswerkmeister.de","avatar":"https://avatars.githubusercontent.com/u/2346599?v=4"},"body":"On 09.04.2018 04:23, Todd Zullinger wrote:\n> Lucas Werkmeister wrote:\n>> Since the --log-destination option was added in 0c591cacb (\"daemon: add\n>> --log-destination=(stderr|syslog|none)\", 2018-02-04) with the explicit\n>> goal of allowing logging to stderr when running in inetd mode, we should\n>> not always redirect stderr to /dev/null in inetd mode, but rather only\n>> when stderr is not being used for logging.\n> \n> Perhaps 's/^F/daemon: f/' on the subject?  (Junio may well\n> already have done so while queueing locally.)\n\nIndeed, sorry! Looks like Junio fixed it on his end (going by the\n“what’s cooking” email), so I won’t reroll. (But I can at least set up a\nlocal commit-msg hook to make sure I don’t forget the subject area again.)\n"}]}