Merge pull request #2118 from jimklimov/nit-warn

Clean up `dummy-ups` code, behavior and logs; added `NUT_DEBUG_PID` option

Author: jimklimov

NEWS.adoc

@@ -139,6 +139,10 @@ as part of https://github.com/networkupstools/nut/issues/1410 solution.
    of UID/GID (everywhere), which makes troubleshooting harder (e.g. lack
    of access to config files or USB device nodes). Now we have it [#1694]
 
+ - A `NUT_DEBUG_PID` envvar (presence) support was added to add current
+   process ID to tags with debug-level identifiers. This may be useful
+   when many NUT daemons write to the same console or log file. [#2118]
+
  - huawei-ups2000 is now known to support more devices, noted in docs and
    for auto-detection [#1448, #1684]
 

UPGRADING.adoc

@@ -128,6 +128,11 @@ Changes from 2.8.0 to 2.8.1
   and exposed in `libnutscan.so` builds in particular - API version for the
   public library was bumped [#317]
 
+- A `NUT_DEBUG_PID` envvar (presence) support was added to add current
+  process ID to tags with debug-level identifiers. This may be useful
+  when many NUT daemons write to the same console or log file, such as
+  in containers/plugins for Home Assistant, storage appliances, etc. [#2118]
+
 - `configure` script, reference init-script and packaging templates updated
   to eradicate `@PIDPATH@/nut` ambiguity in favor of `@ALTPIDPATH@` for the
   unprivileged processes vs. `@PIDPATH@` for those running as root [#1719]

common/common.c

@@ -1519,6 +1519,7 @@ void s_upsdebug_with_errno(int level, const char *fmt, ...)
 {
 	va_list va;
 	char fmt2[LARGEBUF];
+	static int NUT_DEBUG_PID = -1;
 
 	/* Note: Thanks to macro wrapping, we do not quite need this
 	 * test now, but we still need the "level" value to report
@@ -1534,7 +1535,18 @@ void s_upsdebug_with_errno(int level, const char *fmt, ...)
  * can help limit this debug stream quicker, than experimentally picking ;) */
 	if (level > 0) {
 		int ret;
-		ret = snprintf(fmt2, sizeof(fmt2), "[D%d] %s", level, fmt);
+
+		if (NUT_DEBUG_PID < 0) {
+			NUT_DEBUG_PID = (getenv("NUT_DEBUG_PID") != NULL);
+		}
+
+		if (NUT_DEBUG_PID) {
+			/* Note that we re-request PID every time as it can
+			 * change during the run-time (forking etc.) */
+			ret = snprintf(fmt2, sizeof(fmt2), "[D%d:%" PRIiMAX "] %s", level, (intmax_t)getpid(), fmt);
+		} else {
+			ret = snprintf(fmt2, sizeof(fmt2), "[D%d] %s", level, fmt);
+		}
 		if ((ret < 0) || (ret >= (int) sizeof(fmt2))) {
 			syslog(LOG_WARNING, "upsdebug_with_errno: snprintf needed more than %d bytes",
 				LARGEBUF);
@@ -1564,14 +1576,27 @@ void s_upsdebugx(int level, const char *fmt, ...)
 {
 	va_list va;
 	char fmt2[LARGEBUF];
+	static int NUT_DEBUG_PID = -1;
 
 	if (nut_debug_level < level)
 		return;
 
 /* See comments above in upsdebug_with_errno() - they apply here too. */
 	if (level > 0) {
 		int ret;
-		ret = snprintf(fmt2, sizeof(fmt2), "[D%d] %s", level, fmt);
+
+		if (NUT_DEBUG_PID < 0) {
+			NUT_DEBUG_PID = (getenv("NUT_DEBUG_PID") != NULL);
+		}
+
+		if (NUT_DEBUG_PID) {
+			/* Note that we re-request PID every time as it can
+			 * change during the run-time (forking etc.) */
+			ret = snprintf(fmt2, sizeof(fmt2), "[D%d:%" PRIiMAX "] %s", level, (intmax_t)getpid(), fmt);
+		} else {
+			ret = snprintf(fmt2, sizeof(fmt2), "[D%d] %s", level, fmt);
+		}
+
 		if ((ret < 0) || (ret >= (int) sizeof(fmt2))) {
 			syslog(LOG_WARNING, "upsdebugx: snprintf needed more than %d bytes",
 				LARGEBUF);

drivers/dummy-ups.c

@@ -2,6 +2,7 @@
 
    Copyright (C)
        2005 - 2015  Arnaud Quette <http://arnaud.quette.free.fr/contact.html>
+       2014 - 2023  Jim Klimov <jimklimov+nut@gmail.com>
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -47,7 +48,7 @@
 #include "dummy-ups.h"
 
 #define DRIVER_NAME	"Device simulation and repeater driver"
-#define DRIVER_VERSION	"0.16"
+#define DRIVER_VERSION	"0.17"
 
 /* driver description structure */
 upsdrv_info_t upsdrv_info =
@@ -98,7 +99,7 @@ static struct stat	datafile_stat;
 
 static int setvar(const char *varname, const char *val);
 static int instcmd(const char *cmdname, const char *extra);
-static int parse_data_file(TYPE_FD upsfd);
+static int parse_data_file(TYPE_FD arg_upsfd);
 static dummy_info_t *find_info(const char *varname);
 static int is_valid_data(const char* varname);
 static int is_valid_value(const char* varname, const char *value);
@@ -207,6 +208,36 @@ void upsdrv_initinfo(void)
 	dstate_addcmd("load.off");
 }
 
+static int prepare_filepath(char *fn, size_t buflen)
+{
+	/* Note: device_path is a global variable,
+	 * the "port=..." value parsed in main.c */
+	if (device_path[0] == '/'
+#ifdef WIN32
+	||  device_path[1] == ':'	/* "C:\..." */
+#endif
+	) {
+		/* absolute path */
+		return snprintf(fn, buflen, "%s", device_path);
+	} else if (device_path[0] == '.') {
+		/* "./" or "../" e.g. via CLI, relative to current working
+		 * directory of the driver process... at this moment */
+		if (getcwd(fn, buflen)) {
+			return snprintf(fn + strlen(fn), buflen - strlen(fn), "/%s", device_path);
+		} else {
+			return snprintf(fn, buflen, "%s", device_path);
+		}
+	} else {
+		/* assumed to be a filename in NUT config file path
+		 * (possibly under, with direct use of dirname without dots)
+		 * Note that we do not fiddle with file-path separator,
+		 * modern Windows (at least via MinGW/MSYS2) supports
+		 * the POSIX slash.
+		 */
+		return snprintf(fn, buflen, "%s/%s", confpath(), device_path);
+	}
+}
+
 void upsdrv_updateinfo(void)
 {
 	upsdebugx(1, "upsdrv_updateinfo...");
@@ -227,42 +258,35 @@ void upsdrv_updateinfo(void)
 				struct stat	fs;
 				char fn[SMALLBUF];
 
-				if (device_path[0] == '/'
-#ifdef WIN32
-				||  device_path[1] == ':'	/* "C:\..." */
-#endif
-				)
-					snprintf(fn, sizeof(fn), "%s", device_path);
-				else if (device_path[0] == '.')	{
-					/* "./" or "../" e.g. via CLI */
-					if (getcwd(fn, sizeof(fn))) {
-						snprintf(fn + strlen(fn), sizeof(fn) - strlen(fn), "/%s", device_path);
-					} else
-						snprintf(fn, sizeof(fn), "%s", device_path);
-				} else
-					snprintf(fn, sizeof(fn), "%s/%s", confpath(), device_path);
+				prepare_filepath(fn, sizeof(fn));
 
 				/* Determine if file modification timestamp has changed
 				 * since last use (so we would want to re-read it) */
 #ifndef WIN32
-				/* Either successful stat is OK to fill the "fs" struct */
-				if (0 != fstat (upsfd, &fs) && 0 != stat (fn, &fs))
+				/* Either successful stat (zero return) is OK to
+				 * fill the "fs" struct. Note that currently
+				 * "upsfd" is a no-op for files, they are re-opened
+				 * and re-parsed every time so callers can modify
+				 * the data without complications.
+				 */
+				if ( (INVALID_FD(upsfd) || 0 != fstat (upsfd, &fs)) && 0 != stat (fn, &fs))
 #else
 				/* Consider GetFileAttributesEx() for WIN32_FILE_ATTRIBUTE_DATA?
 				 *   https://stackoverflow.com/questions/8991192/check-the-file-size-without-opening-file-in-c/8991228#8991228
 				 */
 				if (0 != stat (fn, &fs))
 #endif
 				{
-					upsdebugx(2, "Can't open %s currently", fn);
+					upsdebugx(2, "%s: MODE_DUMMY_ONCE: Can't stat %s currently", __func__, fn);
 					/* retry ASAP until we get a file */
 					memset(&datafile_stat, 0, sizeof(struct stat));
 					next_update = 1;
 				} else {
 					if (datafile_stat.st_mtime != fs.st_mtime) {
 						upsdebugx(2,
-							"upsdrv_updateinfo: input file was already read once "
-							"to the end, but changed later - re-reading: %s", fn);
+							"%s: MODE_DUMMY_ONCE: input file was already read once "
+							"to the end, but changed later - re-reading: %s",
+							__func__, fn);
 						/* updated file => retry ASAP */
 						next_update = 1;
 						datafile_stat = fs;
@@ -271,7 +295,7 @@ void upsdrv_updateinfo(void)
 			}
 
 			if (ctx == NULL && next_update == -1) {
-				upsdebugx(2, "upsdrv_updateinfo: NO-OP: input file was already read once to the end");
+				upsdebugx(2, "%s: MODE_DUMMY_ONCE: NO-OP: input file was already read once to the end", __func__);
 				dstate_dataok();
 			} else {
 				/* initial parsing interrupted by e.g. TIMER line */
@@ -474,33 +498,25 @@ void upsdrv_initups(void)
 #endif
 		}
 
-		if (device_path[0] == '/'
-#ifdef WIN32
-		||  device_path[1] == ':'	/* "C:\..." */
-#endif
-		)
-			snprintf(fn, sizeof(fn), "%s", device_path);
-		else if (device_path[0] == '.')	{
-			/* "./" or "../" e.g. via CLI */
-			if (getcwd(fn, sizeof(fn))) {
-				snprintf(fn + strlen(fn), sizeof(fn) - strlen(fn), "/%s", device_path);
-			} else
-				snprintf(fn, sizeof(fn), "%s", device_path);
-		} else
-			snprintf(fn, sizeof(fn), "%s/%s", confpath(), device_path);
+		prepare_filepath(fn, sizeof(fn));
 
 		/* Update file modification timestamp (and other data) */
 #ifndef WIN32
-		/* Either successful stat is OK to fill the "datafile_stat" struct */
-		if (0 != fstat (upsfd, &datafile_stat) && 0 != stat (device_path, &datafile_stat))
+		/* Either successful stat (zero return) is OK to fill the
+		 * "datafile_stat" struct. Note that currently "upsfd" is
+		 * a no-op for files, they are re-opened and re-parsed
+		 * every time so callers can modify the data without
+		 * complications.
+		 */
+		if ( (INVALID_FD(upsfd) || 0 != fstat (upsfd, &datafile_stat)) && 0 != stat (fn, &datafile_stat))
 #else
 		/* Consider GetFileAttributesEx() for WIN32_FILE_ATTRIBUTE_DATA?
 		 *   https://stackoverflow.com/questions/8991192/check-the-file-size-without-opening-file-in-c/8991228#8991228
 		 */
-		if (0 != stat (device_path, &datafile_stat))
+		if (0 != stat (fn, &datafile_stat))
 #endif
 		{
-			upsdebugx(2, "Can't open %s (%s) currently", device_path, fn);
+			upsdebugx(2, "%s: Can't stat %s (%s) currently", __func__, device_path, fn);
 		} else {
 			upsdebugx(2, "Located %s for device simulation data: %s", device_path, fn);
 		}
@@ -642,7 +658,7 @@ static dummy_info_t *find_info(const char *varname)
 			return item;
 	}
 
-	upsdebugx(2, "find_info: unknown variable: %s\n", varname);
+	upsdebugx(2, "find_info: unknown variable: %s", varname);
 
 	return NULL;
 }
@@ -719,21 +735,7 @@ static int parse_data_file(TYPE_FD arg_upsfd)
 	{
 		ctx = (PCONF_CTX_t *)xmalloc(sizeof(PCONF_CTX_t));
 
-		if (device_path[0] == '/'
-#ifdef WIN32
-		||  device_path[1] == ':'	/* "C:\..." */
-#endif
-		)
-			snprintf(fn, sizeof(fn), "%s", device_path);
-		else if (device_path[0] == '.')	{
-			/* "./" or "../" e.g. via CLI */
-			if (getcwd(fn, sizeof(fn))) {
-				snprintf(fn + strlen(fn), sizeof(fn) - strlen(fn), "/%s", device_path);
-			} else
-				snprintf(fn, sizeof(fn), "%s", device_path);
-		} else
-			snprintf(fn, sizeof(fn), "%s/%s", confpath(), device_path);
-
+		prepare_filepath(fn, sizeof(fn));
 		pconf_init(ctx, upsconf_err);
 
 		if (!pconf_file_begin(ctx, fn))