[tac_plus] Debugging output cleanup (v2)

Philp Prindeville philipp at redfish-solutions.com
Tue Aug 16 17:27:25 UTC 2016


This is a redux, using a stack-based buffer instead of a malloc'd one.


On 08/12/2016 05:48 PM, Philp Prindeville wrote:
> This fixes the following things:
>
> * dump characters as 2 hex digits, even if high nibble is zero;
> * when displaying hash inputs, consistently show session_id in network 
> byte order;
> * dump hash (and prev_hash) on one line, rather than 1 byte per line;
> * remove extraneous newline
> * use open_memstream() to avoid strcat()'s, and other buffer-to-buffer 
> copies;
> * don't reinvent the printf-wheel. use vfprintf() instead;
>
> Running it here locally.
>
> -Philip
>


-------------- next part --------------
diff -p -urN a/dump.c b/dump.c
--- a/dump.c	2009-07-17 11:34:32.000000000 -0600
+++ b/dump.c	2016-08-12 17:39:31.806521663 -0600
@@ -102,7 +102,7 @@ dump_header(u_char *pak)
     report(LOG_DEBUG, "PACKET: key=%s", session.key ? session.key : "<NULL>");
     report(LOG_DEBUG, "version %d (0x%x), type %d, seq no %d, flags 0x%x",
 	   hdr->version, hdr->version, hdr->type, hdr->seq_no, hdr->flags);
-    report(LOG_DEBUG, "session_id %u (0x%x), Data length %d (0x%x)",
+    report(LOG_DEBUG, "session_id %u (0x%08x), Data length %d (0x%x)",
 	   ntohl(hdr->session_id), ntohl(hdr->session_id),
 	   ntohl(hdr->datalength), ntohl(hdr->datalength));
 
diff -p -urN a/encrypt.c b/encrypt.c
--- a/encrypt.c	2009-07-17 11:34:30.000000000 -0600
+++ b/encrypt.c	2016-08-12 17:39:04.794520994 -0600
@@ -114,19 +114,17 @@ md5_xor(HDR *hdr, u_char *data, char *ke
 	    int k;
 
 	    report(LOG_DEBUG,
-		   "hash: session_id=%u, key=%s, version=%d, seq_no=%d",
-		   session_id, key, version, seq_no);
+		   "hash: session_id=%u (0x%08x), key=%s, version=%d, seq_no=%d",
+		   htonl(session_id), htonl(session_id), key, version, seq_no);
 	    if (prev_hashp) {
 		report(LOG_DEBUG, "prev_hash:");
-		for (k = 0; k < MD5_LEN; k++)
-		    report(LOG_DEBUG, "0x%x", prev_hashp[k]);
+		report_hex(LOG_DEBUG, prev_hashp, MD5_LEN);
 	    } else {
 		report(LOG_DEBUG, "no prev. hash");
 	    }
 
 	    report(LOG_DEBUG, "hash: ");
-	    for (k = 0; k < MD5_LEN; k++)
-		report(LOG_DEBUG, "0x%x", hash[k]);
+	    report_hex(LOG_DEBUG, hash, MD5_LEN);
 	}			/* debug */
 	memcpy(last_hash, hash, MD5_LEN);
 	prev_hashp = last_hash;
@@ -141,7 +139,7 @@ md5_xor(HDR *hdr, u_char *data, char *ke
 	    }
 	    if (debug & DEBUG_XOR_FLAG) {
 		report(LOG_DEBUG,
-		       "data[%d] = 0x%x, xor'ed with hash[%d] = 0x%x -> 0x%x\n",
+		       "data[%d] = 0x%02x, xor'ed with hash[%d] = 0x%02x -> 0x%02x",
 		       i + j,
 		       data[i + j],
 		       j,
diff -p -urN a/report.c b/report.c
--- a/report.c	2009-07-17 11:34:31.000000000 -0600
+++ b/report.c	2016-08-16 11:17:34.354521000 -0600
@@ -58,92 +58,27 @@ report(priority, fmt, va_alist)
     va_dcl				/* no terminating semi-colon */
 #endif
 {
-    char msg[255];		/* temporary string */
-    char *fp, *bufp, *charp;
-    int len, m, i, n;
-    char digits[16];
+    char buf[256];
+    size_t nchars;
+    FILE *mem;
     va_list ap;
 
-    charp = NULL;
-    m = 0;
-
 #ifdef __STDC__
     va_start(ap, fmt);
 #else
     va_start(ap);
 #endif
 
-    /* ensure that msg is never overwritten */
-    n = 255;
-    fp = fmt;
-    len = 0;
-    msg[n-1] = '\0';
-    bufp = msg;
-
-    while (*fp) {
-
-	if (*fp != '%') {
-	    if ((len+1) >= n) {
-		break;
-	    }
-	    *bufp++ = *fp++;
-	    len++;
-	    continue;
-	}
-
-	/* seen a '%' */
-	fp++;
-
-	switch (*fp) {
-
-	case 's':
-	    fp++;
-	    charp = va_arg(ap, char *);
-	    m = strlen(charp);
-	    break;
-
-	case 'u':
-	    fp++;
-	    i = va_arg(ap, uint);
-	    sprintf(digits, "%u", i);
-	    m = strlen(digits);
-	    charp = digits;
-	    break;
-	case 'x':
-	    fp++;
-	    i = va_arg(ap, uint);
-	    sprintf(digits, "%x", i);
-	    m = strlen(digits);
-	    charp = digits;
-	    break;
-	case 'd':
-	    fp++;
-	    i = va_arg(ap, int);
-	    sprintf(digits, "%d", i);
-	    m = strlen(digits);
-	    charp = digits;
-	    break;
-	}
+    mem = fmemopen(buf, sizeof(buf), "w");
 
-	if ((len + m + 1) >= n) {
-	    break;
-	}
-
-	memcpy(bufp, charp, m);
-	bufp += m;
-	len += m;
-	continue;
-    }
-
-    msg[len] = '\0';
-
-    /* check we never overwrote the end of the buffer */
-    if (msg[n-1]) {
-	abort();
-    }
+    vfprintf(mem, fmt, ap);
 
     va_end(ap);
 
+    fflush(mem);
+    nchars = ftell(mem);
+
+    fclose(mem);
 
     if (console) {
 	if (!ostream)
@@ -152,7 +87,8 @@ report(priority, fmt, va_alist)
 	if (ostream) {
 	    if (priority == LOG_ERR)
 		fprintf(ostream, "Error ");
-	    fprintf(ostream, "%s\n", msg);
+	    fwrite(buf, nchars, 1, ostream);
+	    fputc('\n', ostream);
 	} else
 	    syslog(LOG_ERR, "Cannot open /dev/console errno=%d", errno);
     }
@@ -162,30 +98,33 @@ report(priority, fmt, va_alist)
 
 	logfd = open(logfile, O_CREAT | O_WRONLY | O_APPEND, 0644);
 	if (logfd >= 0) {
-	    char buf[512];
 	    time_t t = time(NULL);
 	    char *ct = ctime(&t);
+	    FILE *flog;
 
 	    ct[24] = '\0';
+
 	    tac_lockfd(logfile, logfd);
-	    sprintf(buf, "%s [%ld]: ", ct, (long)getpid());
-	    write(logfd, buf, strlen(buf));
+	    flog = fdopen(logfd, "a");
+
+	    fprintf(flog, "%s [%ld]: ", ct, (long)getpid());
 	    if (priority == LOG_ERR)
-		write(logfd, "Error ", 6);
-	    write(logfd, msg, strlen(msg));
-	    write(logfd, "\n", 1);
-	    close(logfd);
+		fputs("Error ", flog);
+	    fwrite(buf, nchars, 1, flog);
+	    fputc('\n', flog);
+	    fclose(flog);
 	}
     }
 
     if (single) {
-	fprintf(stderr, "%s\n", msg);
+	fwrite(buf, nchars, 1, stderr);
+	fputc('\n', stderr);
     }
 
     if (priority == LOG_ERR)
-	syslog(priority, "Error %s", msg);
+	syslog(priority, "Error %s", buf);
     else
-	syslog(priority, "%s", msg);
+	syslog(priority, "%s", buf);
 }
 
 /* format a hex dump for syslog */
@@ -193,32 +132,36 @@ void
 report_hex(int priority, u_char *p, int len)
 {
     char buf[256];
-    char digit[10];
-    int buflen;
-    int i;
+    size_t nchars;
+    unsigned i;
+    FILE *mem;
 
     if (len <= 0)
 	return;
 
-    buf[0] = '\0';
-    buflen = 0;
-    for (i = 0; i < len && i < 255; i++, p++) {
-
-	sprintf(digit, "0x%x ", *p);
-	strcat(buf, digit);
-	buflen += strlen(digit);
+    mem = fmemopen(buf, sizeof(buf), "w");
+
+    if (len > 255)
+	len = 255;
+
+    for (i = 0; i < len; i++, p++) {
+
+	fprintf(mem, "0x%02x ", *p);
 
-	if (buflen > 75) {
+	if (ftell(mem) > 75) {
+	    fflush(mem);
 	    report(priority, "%s", buf);
-	    buf[0] = '\0';
-	    buflen = 0;
+	    rewind(mem);
 	}
     }
 
-    if (buf[0]) {
+    if (ftell(mem) > 0) {
+	fflush(mem);
 	report(priority, "%s", buf);
     }
 
+    fclose(mem);
+
     return;
 }
 
@@ -227,8 +170,9 @@ void
 report_string(int priority, u_char *p, int len)
 {
     char buf[256];
-    char *bufp = buf;
+    size_t nchars;
     int i, n;
+    FILE *mem;
 
     if (len <= 0)
 	return;
@@ -236,20 +180,21 @@ report_string(int priority, u_char *p, i
     if (len > 255)
 	len = 255;
 
-    for (i = 0; i < len; i++) {
+    mem = fmemopen(buf, sizeof(buf), "w");
+
+    for (i = n = 0; i < len && n < len; i++, p++) {
 	/* ASCII printable, else ... */
 	if (32 <= *p && *p <= 126) {
-	    *bufp++ = *p++;
+	    fputc(*p, mem);
+	    ++n;
 	} else {
-	    n = snprintf(bufp, len - i, " 0x%x ", *p);
-	    if (n >= len - i)
-		break;
-	    bufp += n;
-	    i += n - 1;
-	    p++;
+	    fprintf(mem, " 0x%02x ", *p);
+	    n += 6;
 	}
     }
-    *bufp = '\0';
+
+    fclose(mem);
+
     report(priority, "%s", buf);
 }
 


More information about the tac_plus mailing list