Cleanup zed logging

This is a set of minor cleanup changes related to zed logging:
- Remove the program identity prefix from messages written to stderr
  since systemd already prepends this output with the program name.
- Replace the copy of the program identity string with a ptr reference.
- Replace "pid" with "PID" for consistency in comments & strings.
- Rename the zed_log.c struct _ctx component "level" to "priority".
- Add the LOG_PID option for messages written to syslog.

Signed-off-by: Chris Dunlap <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Issue #2252

4 files changed, 58 insertions, 62 deletions

diff --git a/cmd/zed/zed_conf.c b/cmd/zed/zed_conf.c
index e6f601f3a..cf68c06fe 100644
--- a/cmd/zed/zed_conf.c
+++ b/cmd/zed/zed_conf.c
@@ -103,7 +103,7 @@ zed_conf_destroy(struct zed_conf *zcp)
 	if (zcp->pid_file) {
 		if ((unlink(zcp->pid_file) < 0) && (errno != ENOENT))
 			zed_log_msg(LOG_WARNING,
-			    "Failed to remove pid file \"%s\": %s",
+			    "Failed to remove PID file \"%s\": %s",
 			    zcp->pid_file, strerror(errno));
 	}
 	if (zcp->conf_file)
@@ -450,14 +450,14 @@ zed_conf_write_pid(struct zed_conf *zcp)
 
 	if (!zcp || !zcp->pid_file) {
 		errno = EINVAL;
-		zed_log_msg(LOG_ERR, "Failed to write pid file: %s",
+		zed_log_msg(LOG_ERR, "Failed to write PID file: %s",
 		    strerror(errno));
 		return (-1);
 	}
 	n = strlcpy(dirbuf, zcp->pid_file, sizeof (dirbuf));
 	if (n >= sizeof (dirbuf)) {
 		errno = ENAMETOOLONG;
-		zed_log_msg(LOG_WARNING, "Failed to write pid file: %s",
+		zed_log_msg(LOG_WARNING, "Failed to write PID file: %s",
 		    strerror(errno));
 		return (-1);
 	}
@@ -479,13 +479,13 @@ zed_conf_write_pid(struct zed_conf *zcp)
 	umask(mask);
 
 	if (!fp) {
-		zed_log_msg(LOG_WARNING, "Failed to open pid file \"%s\": %s",
+		zed_log_msg(LOG_WARNING, "Failed to open PID file \"%s\": %s",
 		    zcp->pid_file, strerror(errno));
 	} else if (fprintf(fp, "%d\n", (int) getpid()) == EOF) {
-		zed_log_msg(LOG_WARNING, "Failed to write pid file \"%s\": %s",
+		zed_log_msg(LOG_WARNING, "Failed to write PID file \"%s\": %s",
 		    zcp->pid_file, strerror(errno));
 	} else if (fclose(fp) == EOF) {
-		zed_log_msg(LOG_WARNING, "Failed to close pid file \"%s\": %s",
+		zed_log_msg(LOG_WARNING, "Failed to close PID file \"%s\": %s",
 		    zcp->pid_file, strerror(errno));
 	} else {
 		return (0);
@@ -564,7 +564,7 @@ zed_conf_open_state(struct zed_conf *zcp)
 			    zcp->state_file);
 		} else if (pid > 0) {
 			zed_log_msg(LOG_WARNING,
-			    "Found pid %d bound to state file \"%s\"",
+			    "Found PID %d bound to state file \"%s\"",
 			    pid, zcp->state_file);
 		} else {
 			zed_log_msg(LOG_WARNING,
diff --git a/cmd/zed/zed_file.c b/cmd/zed/zed_file.c
index 7b77345d1..3d62f9fe4 100644
--- a/cmd/zed/zed_file.c
+++ b/cmd/zed/zed_file.c
@@ -145,7 +145,7 @@ zed_file_unlock(int fd)
 /*
  * Test whether an exclusive advisory lock could be obtained for the open
  *   file descriptor [fd].
- * Return 0 if the file is not locked, >0 for the pid of another process
+ * Return 0 if the file is not locked, >0 for the PID of another process
  *   holding a conflicting lock, or -1 on error (with errno set).
  */
 pid_t
diff --git a/cmd/zed/zed_log.c b/cmd/zed/zed_log.c
index 2a787e357..c2bc25edc 100644
--- a/cmd/zed/zed_log.c
+++ b/cmd/zed/zed_log.c
@@ -33,38 +33,40 @@
 #include <syslog.h>
 #include "zed_log.h"
 
-#define	ZED_LOG_MAX_ID_LEN	64
 #define	ZED_LOG_MAX_LOG_LEN	1024
 
 static struct {
 	unsigned do_stderr:1;
 	unsigned do_syslog:1;
-	int level;
-	char id[ZED_LOG_MAX_ID_LEN];
+	const char *identity;
+	int priority;
 	int pipe_fd[2];
 } _ctx;
 
+/*
+ * Initialize the logging subsystem.
+ */
 void
 zed_log_init(const char *identity)
 {
-	const char *p;
-
 	if (identity) {
-		p = (p = strrchr(identity, '/')) ? p + 1 : identity;
-		strlcpy(_ctx.id, p, sizeof (_ctx.id));
+		const char *p = strrchr(identity, '/');
+		_ctx.identity = (p != NULL) ? p + 1 : identity;
 	} else {
-		_ctx.id[0] = '\0';
+		_ctx.identity = NULL;
 	}
 	_ctx.pipe_fd[0] = -1;
 	_ctx.pipe_fd[1] = -1;
 }
 
+/*
+ * Shutdown the logging subsystem.
+ */
 void
-zed_log_fini()
+zed_log_fini(void)
 {
-	if (_ctx.do_syslog) {
-		closelog();
-	}
+	zed_log_stderr_close();
+	zed_log_syslog_close();
 }
 
 /*
@@ -158,78 +160,72 @@ zed_log_pipe_wait(void)
 	}
 }
 
+/*
+ * Start logging messages at the syslog [priority] level or higher to stderr.
+ *   Refer to syslog(3) for valid priority values.
+ */
 void
-zed_log_stderr_open(int level)
+zed_log_stderr_open(int priority)
 {
 	_ctx.do_stderr = 1;
-	_ctx.level = level;
+	_ctx.priority = priority;
 }
 
+/*
+ * Stop logging messages to stderr.
+ */
 void
 zed_log_stderr_close(void)
 {
-	_ctx.do_stderr = 0;
+	if (_ctx.do_stderr)
+		_ctx.do_stderr = 0;
 }
 
+/*
+ * Start logging messages to syslog.
+ *   Refer to syslog(3) for valid option/facility values.
+ */
 void
 zed_log_syslog_open(int facility)
 {
-	const char *identity;
-
 	_ctx.do_syslog = 1;
-	identity = (_ctx.id[0] == '\0') ? NULL : _ctx.id;
-	openlog(identity, LOG_NDELAY, facility);
+	openlog(_ctx.identity, LOG_NDELAY | LOG_PID, facility);
 }
 
+/*
+ * Stop logging messages to syslog.
+ */
 void
 zed_log_syslog_close(void)
 {
-	_ctx.do_syslog = 0;
-	closelog();
+	if (_ctx.do_syslog) {
+		_ctx.do_syslog = 0;
+		closelog();
+	}
 }
 
+/*
+ * Auxiliary function to log a message to syslog and/or stderr.
+ */
 static void
 _zed_log_aux(int priority, const char *fmt, va_list vargs)
 {
 	char buf[ZED_LOG_MAX_LOG_LEN];
-	char *syslogp;
-	char *p;
-	int len;
 	int n;
 
-	assert(fmt != NULL);
-
-	syslogp = NULL;
-	p = buf;
-	len = sizeof (buf);
+	if (!fmt)
+		return;
 
-	if (_ctx.id[0] != '\0') {
-		n = snprintf(p, len, "%s: ", _ctx.id);
-		if ((n < 0) || (n >= len)) {
-			p += len - 1;
-			len = 0;
-		} else {
-			p += n;
-			len -= n;
-		}
-	}
-	if ((len > 0) && fmt) {
-		syslogp = p;
-		n = vsnprintf(p, len, fmt, vargs);
-		if ((n < 0) || (n >= len)) {
-			p += len - 1;
-			len = 0;
-		} else {
-			p += n;
-			len -= n;
-		}
+	n = vsnprintf(buf, sizeof (buf), fmt, vargs);
+	if ((n < 0) || (n >= sizeof (buf))) {
+		buf[sizeof (buf) - 2] = '+';
+		buf[sizeof (buf) - 1] = '\0';
 	}
-	*p = '\0';
 
-	if (_ctx.do_syslog && syslogp)
-		syslog(priority, "%s", syslogp);
+	if (_ctx.do_syslog)
+		syslog(priority, "%s", buf);
 
-	if (_ctx.do_stderr && priority <= _ctx.level)
+	if (_ctx.do_stderr && (priority <= _ctx.priority))
 		fprintf(stderr, "%s\n", buf);
 }
 
diff --git a/cmd/zed/zed_log.h b/cmd/zed/zed_log.h
index 767c4c7db..d963089d0 100644
--- a/cmd/zed/zed_log.h
+++ b/cmd/zed/zed_log.h
@@ -41,7 +41,7 @@ void zed_log_pipe_close_writes(void);
 
 void zed_log_pipe_wait(void);
 
-void zed_log_stderr_open(int level);
+void zed_log_stderr_open(int priority);
 
 void zed_log_stderr_close(void);