Erik Faye-Lund [off-list ref] writes:
quoted hunk
Report incoming connections from the process that
accept() the connection instead of the handling
process.
This enables "Connection from"-reporting on
Windows, where getpeername(0, ...) consistently
fails.
Signed-off-by: Erik Faye-Lund <redacted>
---
daemon.c | 72 ++++++++++++++++++++++++++++++-------------------------------
1 files changed, 35 insertions(+), 37 deletions(-)
diff --git a/daemon.c b/daemon.c
index 8a44fb9..1574f75 100644
--- a/daemon.c
+++ b/daemon.c
@@ -516,38 +516,11 @@ static void parse_host_arg(char *extra_args, int buflen)
}
-static int execute(struct sockaddr *addr)
+static int execute(void)
{
-...
- }
- loginfo("Connection from %s:%d", addrbuf, port);
- setenv("REMOTE_ADDR", addrbuf, 1);...
+ else {
+ loginfo("[%"PRIuMAX"] Connection from %s:%d",
+ (uintmax_t)cld.pid, addrstr, port);
add_child(&cld, addr, addrlen);
Hmm, loginfo() calls logreport() and adds the process information as
necessary to the output. Wouldn't this patch give the pid information
twice?
On Thu, Oct 14, 2010 at 12:55 AM, Junio C Hamano [off-list ref] wrote:
Erik Faye-Lund [off-list ref] writes:
quoted
Report incoming connections from the process that
accept() the connection instead of the handling
process.
This enables "Connection from"-reporting on
Windows, where getpeername(0, ...) consistently
fails.
Signed-off-by: Erik Faye-Lund <redacted>
---
daemon.c | 72 ++++++++++++++++++++++++++++++-------------------------------
1 files changed, 35 insertions(+), 37 deletions(-)
diff --git a/daemon.c b/daemon.c
index 8a44fb9..1574f75 100644
--- a/daemon.c
+++ b/daemon.c
@@ -516,38 +516,11 @@ static void parse_host_arg(char *extra_args, int buflen)
}
-static int execute(struct sockaddr *addr)
+static int execute(void)
{
-...
- }
- loginfo("Connection from %s:%d", addrbuf, port);
- setenv("REMOTE_ADDR", addrbuf, 1);
...
+ else {
+ loginfo("[%"PRIuMAX"] Connection from %s:%d",
+ (uintmax_t)cld.pid, addrstr, port);
add_child(&cld, addr, addrlen);
Hmm, loginfo() calls logreport() and adds the process information as
necessary to the output. Wouldn't this patch give the pid information
twice?
Close, but not quite. logreport() reports the current PID, while this
call to loginfo reports the PID of the child process. So two
non-identical PIDs are reported.
The output becomes something like this:
[6408] [3868] Connection from [::1]:55801
[3868] Extended attributes (16 bytes) exist <host=localhost>
[3868] Request upload-pack for '/some-repo.git'
[3868] '/some-repo.git' does not appear to be a git repository
[6408] [1876] Connection from [::1]:57311
[1876] Extended attributes (16 bytes) exist <host=localhost>
[1876] Request upload-pack for '/some-repo.git'
[1876] '/some-repo.git' does not appear to be a git repository
Erik Faye-Lund [off-list ref] writes:
quoted
Hmm, loginfo() calls logreport() and adds the process information as
necessary to the output. Wouldn't this patch give the pid information
twice?
Close, but not quite. logreport() reports the current PID, while this
call to loginfo reports the PID of the child process. So two
non-identical PIDs are reported.
I know that; I was questioning if that change to the log output is really
what we want. I do not deeply care myself, but people with scripts that
read logs might.
The output becomes something like this:
[6408] [3868] Connection from [::1]:55801
[3868] Extended attributes (16 bytes) exist <host=localhost>
[3868] Request upload-pack for '/some-repo.git'
[3868] '/some-repo.git' does not appear to be a git repository
[6408] [1876] Connection from [::1]:57311
[1876] Extended attributes (16 bytes) exist <host=localhost>
[1876] Request upload-pack for '/some-repo.git'
[1876] '/some-repo.git' does not appear to be a git repository
On Sun, Oct 17, 2010 at 6:43 AM, Junio C Hamano [off-list ref] wrote:
Erik Faye-Lund [off-list ref] writes:
quoted
quoted
Hmm, loginfo() calls logreport() and adds the process information as
necessary to the output. Wouldn't this patch give the pid information
twice?
Close, but not quite. logreport() reports the current PID, while this
call to loginfo reports the PID of the child process. So two
non-identical PIDs are reported.
I know that; I was questioning if that change to the log output is really
what we want. I do not deeply care myself, but people with scripts that
read logs might.
I could do something like this, but then we lose the port information.
Perhaps I could add a REMOTE_PORT environment variable to solve that?
diff --git a/daemon.c b/daemon.c
index 589bd04..3d18899 100644
--- a/daemon.c
+++ b/daemon.c
@@ -522,6 +522,10 @@ static int execute(void)
{
static char line[1000];
int pktlen, len, i;
+ char *addr = getenv("REMOTE_ADDR");
+
+ if (addr)
+ loginfo("Connection from %s", addr);
alarm(init_timeout ? init_timeout : timeout);
pktlen = packet_read_line(0, line, sizeof(line));@@ -702,11 +706,8 @@ static void handle(int incoming, struct sockaddr
*addr, socklen_t addrlen)
if (start_command(&cld))
logerror("unable to fork");
- else {
- loginfo("[%"PRIuMAX"] Connection from %s:%d",
- (uintmax_t)cld.pid, addrstr, port);
+ else
add_child(&cld, addr, addrlen);
- }
close(incoming);
}