From: Junio C Hamano <junkio@cox.net>
Date: Thu, 22 Sep 2005 23:42:39 GMT
Subject: Re: [PATCH] Verbose git-daemon logging
Message-ID: <7vd5n0lle8.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <20050922092528.GB21019@pasky.or.cz>

Petr Baudis <pasky@suse.cz> writes:

> What would be knowing parent PID be useful for? (It's not really the PID
> number that's useful anyway (at least mostly), it's just a good unique
> identifier to distinguish between several simultaneous sessions.)

Maybe running two daemons serving two sets of repos?  But anyway
what you have look more consistent so let's keep that.

> Fixed up patch follows.

Thanks.

> +static void logreport(const char *err, va_list params)
> +{
> +	/* We should do a single write so that it is atomic and output
> +	 * of several processes does not get intermangled. */
> +	char buf[1024];
> +	int buflen;
> +	int maxlen, msglen;
> +
> +	buflen = snprintf(buf, sizeof(buf), "[%d] ", getpid());
> +
> +	maxlen = sizeof(buf) - buflen - 1;
> +	msglen = vsnprintf(buf + buflen, maxlen, err, params);
> +	if (msglen > maxlen)
> +		msglen = maxlen;
> +	else if (msglen < 0)
> +		msglen = -1; /* Protect against weird return values. */
> +	buflen += msglen;
> +
> +	buf[buflen++] = '\n';
> +	buf[buflen] = '\0';
> +
> +	fputs(buf, stderr);
> +}

The tail part still looks odd here.  Maybe something like this?

+static void logreport(const char *err, va_list params)
+{
+	/* We should do a single write so that it is atomic and output
+	 * of several processes does not get intermangled. */
+	char buf[1024];
+	int buflen;
+	int maxlen, msglen;
+
+	/* sizeof(buf) should be big enough for "[pid] \n" */
+	buflen = snprintf(buf, sizeof(buf), "[%d] ", getpid());
+
+	maxlen = sizeof(buf) - buflen - 1; /* -1 for our own LF */
+	msglen = vsnprintf(buf + buflen, maxlen, err, params);
+
+	/* maxlen counted our own LF but also counts space given to
+	 * vsnprintf for the terminating NUL.  We want to make sure that
+	 * we have space for our own LF and NUL after the "meat" of the
+	 * message, so truncate it at maxlen - 1.
+	 */
+	if (msglen > maxlen - 1)
+		msglen = maxlen - 1;
+	else if (msglen < 0)
+		msglen = 0; /* Protect against weird return values. */
+	buflen += msglen;
+
+	buf[buflen++] = '\n';
+	buf[buflen] = '\0';
+
+	fputs(buf, stderr);
+}