Jan Engelhardt <jengelh@xxxxxxxxxx> writes:
> parent v1.6.1
>
> git-daemon: single-line logs
Please drop these two needless lines when/if you are submitting patches
for inclusion..
> Having just a single line per connection attempt, much like Apache
> httpd2 access logs, makes log parsing much easier, especially when
> just glancing over it non-automated.
While I like the motivation, and I wish the log were as terse as possible
from the day one, I think changing the output format unconditionally like
this patch does is a horrible idea. I'd expect there are many people who
already have their infrastructure set up to parse the current output; this
patch actively breaks things for them, doesn't it?
> @@ -295,12 +295,13 @@ static int git_daemon_config(const char
> return 0;
> }
>
> -static int run_service(char *dir, struct daemon_service *service)
> +static int run_service(char *dir, struct daemon_service *service,
> + const char *origin, const char *vhost)
> {
> const char *path;
> int enabled = service->enabled;
>
> - loginfo("Request %s for '%s'", service->name, dir);
> + loginfo("%s->%s %s \"%s\"\n", origin, vhost, service->name, dir);
Mental note. You are adding origin and vhost probably because you are
losing them from elsewhere..
> @@ -507,10 +508,10 @@ static void parse_extra_args(char *extra
> static int execute(struct sockaddr *addr)
> {
> static char line[1000];
> + char addrbuf[256] = "";
> int pktlen, len, i;
>
> if (addr) {
> - char addrbuf[256] = "";
> int port = -1;
>
> if (addr->sa_family == AF_INET) {
> @@ -529,7 +530,6 @@ static int execute(struct sockaddr *addr
> port = ntohs(sin6_addr->sin6_port);
> #endif
> }
> - loginfo("Connection from %s:%d", addrbuf, port);
Mental note. Port is not logged anymore here.
> @@ -541,10 +541,6 @@ static int execute(struct sockaddr *addr
> alarm(0);
>
> len = strlen(line);
> - if (pktlen != len)
> - loginfo("Extended attributes (%d bytes) exist <%.*s>",
> - (int) pktlen - len,
> - (int) pktlen - len, line + len + 1);
Mental note. XA are not logged here anymore.
> @@ -569,7 +565,8 @@ static int execute(struct sockaddr *addr
> * Note: The directory here is probably context sensitive,
> * and might depend on the actual service being performed.
> */
> - return run_service(line + namelen + 5, s);
> + return run_service(line + namelen + 5, s,
> + addrbuf, hostname);
> }
> }
So not just you are changing the format, but you are losing information as
well.
By the way, I think hostname has already been freed and NULLed at this
call site. Aren't you getting entries like:
192.168.0.1->(null) upload-pack "/pub/git.git"
in your log?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
