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

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

[Andreas Ericsson]

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
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

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

[Junio C Hamano]

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.

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

[Andreas Ericsson]

Junio C Hamano wrote:
> 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

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

[Junio C Hamano]

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>

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

[H. Peter Anvin]

Junio C Hamano wrote:
> 
> 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