threads / patch / 2501

patchgit-daemon: --inetd implies --syslog

Subject: [PATCH] git-daemon: --inetd implies --syslog

## tl;dr

5 messages between Nov 14, 2005 and Nov 14, 2005. Diffs are folded; open one to read it.

replies: 4people: 4as markdown or json

Andreas Ericsson· Nov 14, 2005, 16:41 UTC · lore
Otherwise nothing is logged anywhere, which is a Bad Thing.
Signed-off-by: Andreas Ericsson <ae@op5.se>
---
 Documentation/git-daemon.txt |    2 +-
 daemon.c                     |    6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

applies-to: 963d67921d6f65c08d51a8c93811f03274c34703 9fac415ad7ec34c21b20b3c349872304869505df

Show changes to 2 files +4 −4

Documentation/git-daemon.txt, daemon.c

diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
index 67c5f22..3783858 100644
--- a/Documentation/git-daemon.txt
+++ b/Documentation/git-daemon.txt
@@ -35,7 +35,7 @@ OPTIONS
 	do not have the 'git-daemon-export-ok' file.
 
 --inetd::
-	Have the server run as an inetd service.
+	Have the server run as an inetd service. Implies --syslog.
 
 --port::
 	Listen on an alternative port.
diff --git a/daemon.c b/daemon.c
index c3f8641..50a3396 100644
--- a/daemon.c
+++ b/daemon.c
@@ -627,9 +627,9 @@ int main(int argc, char **argv)
 	}
 
 	if (inetd_mode) {
-		fclose(stderr); //FIXME: workaround
+		log_syslog = 1;
 		return execute();
-	} else {
-		return serve(port);
 	}
+
+	return serve(port);
 }
---
0.99.9.GIT
Junio C Hamano· Nov 14, 2005, 20:54 UTC · re: Andreas Ericsson · lore

Re: [PATCH] git-daemon: --inetd implies --syslog

exon@op5.se (Andreas Ericsson) writes:
> Otherwise nothing is logged anywhere, which is a Bad Thing.
>
> Signed-off-by: Andreas Ericsson <ae@op5.se>

Thanks; I am OK with what you are trying to do with this change, except that:

 - I suspect closing stderr is still needed (the "workaround"
   was about inetd connection sending output to both fd 1 and 2
   to the client, which would corrupt the protocol conversation
   when exec'ed program writes anything to its standard error
   stream).
 - I would have preferred the removal of needless else as a
   separate cleanup patch (this is minor).

Peter, I am CC'ing this to you because I do not know how you run the daemon on kernel.org machines -- I suspect you use --syslog already and am hoping you are OK with this change.

Andreas Ericsson· Nov 14, 2005, 21:23 UTC · re: Junio C Hamano · lore

Re: [PATCH] git-daemon: --inetd implies --syslog

Junio C Hamano wrote:
Show 17 quoted lines
> exon@op5.se (Andreas Ericsson) writes:
> 
> 
>>Otherwise nothing is logged anywhere, which is a Bad Thing.
>>
>>Signed-off-by: Andreas Ericsson <ae@op5.se>
> 
> 
> Thanks; I am OK with what you are trying to do with this change,
> except that:
> 
>  - I suspect closing stderr is still needed (the "workaround"
>    was about inetd connection sending output to both fd 1 and 2
>    to the client, which would corrupt the protocol conversation
>    when exec'ed program writes anything to its standard error
>    stream).
> 
That shouldn't be a problem because;
1) handle() dupes the connected socket to stdin and stdout, but not stderr.
2) A program sending output to stderr() fails (well, *should* be either 
failing or silent), so it's most likely not sane to continue doing 
things anyway. This assumes that no client prints anything to stderr 
that can be interpreted as "real" protocol data, ofcourse.
>  - I would have preferred the removal of needless else as a
>    separate cleanup patch (this is minor).
> 
Sorry. I think I missed that part when I glanced at the diff output.
-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231
Junio C Hamano· Nov 14, 2005, 22:01 UTC · re: Andreas Ericsson · lore

Re: [PATCH] git-daemon: --inetd implies --syslog

Andreas Ericsson <ae@op5.se> writes:
> That shouldn't be a problem because;
> 1) handle() dupes the connected socket to stdin and stdout, but not stderr.
I am afraid that
$ git whatchanged -s -S'FIXME' daemon.c
tells me otherwise.  Whatever we do in handle() is too late to matter.
diff-tree 7c3693f1f240e17772c864fad35bc16226038fc8 (from 6a2e50f9dfdca7ac572bbe30dd0efdf19ca250f6)
Author: lars.doelle@on-line.de <lars.doelle@on-line.de>
Date:   Thu Sep 8 03:50:01 2005 +0200
    [PATCH] git-daemon --inetd
    
    git-daemon using inetd. does not work properly. inetd routes stderr onto the
    network line just like stdout, which was apparently not expected to be so.
    
    As the result of this, the stream is closed by the receiver, because some
    "Packing %d objects\n" originating from pack_objects is first reported over
    the line instead of the expected pack_header, and so the SIGNATURE test
    fails.  Here is a workaround.
    
    Signed-off-by: Junio C Hamano <junkio@cox.net>
H. Peter Anvin· Nov 14, 2005, 23:59 UTC · re: Junio C Hamano · lore

Re: [PATCH] git-daemon: --inetd implies --syslog

Junio C Hamano wrote:
Show 16 quoted lines
> 
> Thanks; I am OK with what you are trying to do with this change,
> except that:
> 
>  - I suspect closing stderr is still needed (the "workaround"
>    was about inetd connection sending output to both fd 1 and 2
>    to the client, which would corrupt the protocol conversation
>    when exec'ed program writes anything to its standard error
>    stream).
> 
>  - I would have preferred the removal of needless else as a
>    separate cleanup patch (this is minor).
> 
> Peter, I am CC'ing this to you because I do not know how you run
> the daemon on kernel.org machines -- I suspect you use --syslog
> already and am hoping you are OK with this change.
Yeah, I use --inetd --syslog.
	-hpa

← back to recent threads