Merge pull request networkupstools#2552 from jimklimov/issue-266-intro

Update log messages and comments in "state" machinery

common/state.c

@@ -4,6 +4,7 @@
 	2003	Russell Kroll <[email protected]>
 	2008	Arjen de Korte <[email protected]>
 	2012	Arnaud Quette <[email protected]>
+	2020-2024	Jim Klimov <[email protected]>
 
    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
@@ -295,6 +296,7 @@ int state_setinfo(st_tree_t **nptr, const char *var, const char *val)
 
 		/* changes should be ignored */
 		if (node->flags & ST_FLAG_IMMUTABLE) {
+			upsdebugx(6, "%s: not changing immutable variable [%s]", __func__, var);
 			return 0;	/* no change */
 		}
 
@@ -357,8 +359,8 @@ int state_addenum(st_tree_t *root, const char *var, const char *val)
 	sttmp = state_tree_find(root, var);
 
 	if (!sttmp) {
-		upslogx(LOG_ERR, "state_addenum: base variable (%s) "
-			"does not exist", var);
+		upslogx(LOG_ERR, "%s: base variable (%s) "
+			"does not exist", __func__, var);
 		return 0;	/* failed */
 	}
 
@@ -400,17 +402,17 @@ int state_addrange(st_tree_t *root, const char *var, const int min, const int ma
 
 	/* sanity check */
 	if (min > max) {
-		upslogx(LOG_ERR, "state_addrange: min is superior to max! (%i, %i)",
-			min, max);
+		upslogx(LOG_ERR, "%s: min is superior to max! (%i, %i)",
+			__func__, min, max);
 		return 0;
 	}
 
 	/* find the tree node for var */
 	sttmp = state_tree_find(root, var);
 
 	if (!sttmp) {
-		upslogx(LOG_ERR, "state_addrange: base variable (%s) "
-			"does not exist", var);
+		upslogx(LOG_ERR, "%s: base variable (%s) "
+			"does not exist", __func__, var);
 		return 0;	/* failed */
 	}
 
@@ -427,8 +429,8 @@ int state_setaux(st_tree_t *root, const char *var, const char *auxs)
 	sttmp = state_tree_find(root, var);
 
 	if (!sttmp) {
-		upslogx(LOG_ERR, "state_addenum: base variable (%s) "
-			"does not exist", var);
+		upslogx(LOG_ERR, "%s: base variable (%s) "
+			"does not exist", __func__, var);
 		return -1;	/* failed */
 	}
 
@@ -524,8 +526,8 @@ void state_setflags(st_tree_t *root, const char *var, size_t numflags, char **fl
 	sttmp = state_tree_find(root, var);
 
 	if (!sttmp) {
-		upslogx(LOG_ERR, "state_setflags: base variable (%s) "
-			"does not exist", var);
+		upslogx(LOG_ERR, "%s: base variable (%s) "
+			"does not exist", __func__, var);
 		return;
 	}
 
@@ -549,7 +551,7 @@ void state_setflags(st_tree_t *root, const char *var, size_t numflags, char **fl
 			continue;
 		}
 
-		upsdebugx(2, "Unrecognized flag [%s]", flag[i]);
+		upsdebugx(2, "%s: Unrecognized flag [%s]", __func__, flag[i]);
 	}
 }
 

docs/net-protocol.txt

@@ -142,7 +142,7 @@ Response:
 - 'ENUM': an enumerated type, which supports a few specific values
 - 'STRING:n': this is a string of maximum length n
 - 'RANGE': this is an numeric, either integer or float, comprised in the
-range (see LIST RANGE)
+  range (see LIST RANGE)
 - 'NUMBER': this is a simple numeric value, either integer or float
 
 ENUM, STRING and RANGE are usually associated with RW, but not always.
@@ -153,7 +153,8 @@ float.
 Note that float values are expressed using decimal (base 10) english-based
 representation, so using a dot, in non-scientific notation.  So hexadecimal,
 exponents, and comma for thousands separator are forbidden.
-For example: "1200.20" is valid, while "1,200.20" and "1200,20" are invalid.
+For example: "1200.20" is valid, while "1,200.20" and "1200,20" and "1.2e4"
+are invalid.
 
 
 This replaces the old "VARTYPE" command.

docs/sock-protocol.txt

@@ -108,10 +108,14 @@ SETFLAGS
 
 Note that this command takes a variable number of arguments, as multiple
 flags are supported.  Also note that they are not crammed together in
-"", since "RW STRING" would mean something completely different.
+"" quotes, since "RW STRING" would mean something completely different.
 
 This also replaces any previous flags for a given variable.
 
+Currently supported flags include `RW`, `STRING` and `NUMBER`
+(detailed in the NUT Network Protocol documentation); unrecognized values
+are quietly ignored.
+
 ADDCMD
 ~~~~~~
 

drivers/dstate.c

@@ -282,7 +282,7 @@ static void send_to_all(const char *fmt, ...)
 
 		result = WriteFile (conn->fd, buf, buflen, &bytesWritten, NULL);
 		if( result == 0 ) {
-			upsdebugx(2, "write failed on handle %p, disconnecting", conn->fd);
+			upsdebugx(2, "%s: write failed on handle %p, disconnecting", __func__, conn->fd);
 			sock_disconnect(conn);
 			continue;
 		}
@@ -301,7 +301,7 @@ static void send_to_all(const char *fmt, ...)
 				"handle %p failed (ret=%" PRIiSIZE "), disconnecting: %s",
 				__func__, buflen, conn->fd, ret, strerror(errno));
 #endif
-			upsdebugx(6, "failed write: %s", buf);
+			upsdebugx(6, "%s: failed write: %s", __func__, buf);
 
 			sock_disconnect(conn);
 
@@ -427,7 +427,7 @@ static int send_to_one(conn_t *conn, const char *fmt, ...)
 			"handle %p failed (ret=%" PRIiSIZE "), disconnecting: %s",
 			__func__, buflen, conn->fd, ret, strerror(errno));
 #endif
-		upsdebugx(6, "failed write: %s", buf);
+		upsdebugx(6, "%s: failed write: %s", __func__, buf);
 		sock_disconnect(conn);
 
 		/* TOTHINK: Maybe fallback elsewhere in other cases? */
@@ -475,7 +475,7 @@ static void sock_connect(TYPE_FD sock)
 	fd = accept(sock, (struct sockaddr *) &sa, &salen);
 
 	if (INVALID_FD(fd)) {
-		upslog_with_errno(LOG_ERR, "accept on unix fd failed");
+		upslog_with_errno(LOG_ERR, "%s: accept on unix fd failed", __func__);
 		return;
 	}
 
@@ -491,15 +491,15 @@ static void sock_connect(TYPE_FD sock)
 		ret = fcntl(fd, F_GETFL, 0);
 
 		if (ret < 0) {
-			upslog_with_errno(LOG_ERR, "fcntl get on unix fd failed");
+			upslog_with_errno(LOG_ERR, "%s: fcntl get on unix fd failed", __func__);
 			close(fd);
 			return;
 		}
 
 		ret = fcntl(fd, F_SETFL, ret | O_NDELAY);
 
 		if (ret < 0) {
-			upslog_with_errno(LOG_ERR, "fcntl set O_NDELAY on unix fd failed");
+			upslog_with_errno(LOG_ERR, "%s: fcntl set O_NDELAY on unix fd failed", __func__);
 			close(fd);
 			return;
 		}
@@ -581,9 +581,9 @@ static void sock_connect(TYPE_FD sock)
 	connhead = conn;
 
 #ifndef WIN32
-	upsdebugx(3, "new connection on fd %d", fd);
+	upsdebugx(3, "%s: new connection on fd %d", __func__, fd);
 #else
-	upsdebugx(3, "new connection on handle %p", sock);
+	upsdebugx(3, "%s: new connection on handle %p", __func__, sock);
 #endif
 
 }
@@ -685,8 +685,8 @@ static int sock_arg(conn_t *conn, size_t numarg, char **arg)
 	char *sockfn = pipename;	/* Just for the report below; not a global var in WIN32 builds */
 #endif
 
-	upsdebugx(6, "Driver on %s is now handling %s with %" PRIuSIZE " args",
-		sockfn, numarg ? arg[0] : "<skipped: no command>", numarg);
+	upsdebugx(6, "%s: Driver on %s is now handling %s with %" PRIuSIZE " args",
+		__func__, sockfn, numarg ? arg[0] : "<skipped: no command>", numarg);
 
 	if (numarg < 1) {
 		return 0;
@@ -1037,9 +1037,9 @@ char * dstate_init(const char *prog, const char *devname)
 	sockfd = sock_open(sockname);
 
 #ifndef WIN32
-	upsdebugx(2, "dstate_init: sock %s open on fd %d", sockname, sockfd);
+	upsdebugx(2, "%s: sock %s open on fd %d", __func__, sockname, sockfd);
 #else
-	upsdebugx(2, "dstate_init: sock %s open on handle %p", sockname, sockfd);
+	upsdebugx(2, "%s: sock %s open on handle %p", __func__, sockname, sockfd);
 #endif
 
 	/* NOTE: Caller must free this string */
@@ -1112,7 +1112,7 @@ int dstate_poll_fds(struct timeval timeout, TYPE_FD arg_extrafd)
 			break;
 
 		default:
-			upslog_with_errno(LOG_ERR, "select unix sockets failed");
+			upslog_with_errno(LOG_ERR, "%s: select unix sockets failed", __func__);
 		}
 
 		return overrun;
@@ -1189,7 +1189,7 @@ int dstate_poll_fds(struct timeval timeout, TYPE_FD arg_extrafd)
 	}
 
 	if (ret == WAIT_FAILED) {
-		upslog_with_errno(LOG_ERR, "waitfor failed");
+		upslog_with_errno(LOG_ERR, "%s: waitfor failed", __func__);
 		return overrun;
 	}
 
@@ -1390,7 +1390,7 @@ void dstate_setaux(const char *var, long aux)
 	sttmp = state_tree_find(dtree_root, var);
 
 	if (!sttmp) {
-		upslogx(LOG_ERR, "dstate_setaux: base variable (%s) does not exist", var);
+		upslogx(LOG_ERR, "%s: base variable (%s) does not exist", __func__, var);
 		return;
 	}
 

drivers/main.c

@@ -269,7 +269,9 @@ void dparam_setinfo(const char *var, const char *val)
 {
 	char	vtmp[SMALLBUF];
 
-	/* store these in dstate for debugging and other help */
+	/* store these in dstate for debugging and other help
+	 * note these are not immutable since we can reload
+	 */
 	if (val) {
 		snprintf(vtmp, sizeof(vtmp), "driver.parameter.%s", var);
 		dstate_setinfo(vtmp, "%s", val);
@@ -304,6 +306,10 @@ void storeval(const char *var, char *val)
 		/* NOTE: No regard for VAR_SENSITIVE here */
 		dstate_setinfo(var+9, "%s", val);
 		dstate_setflags(var+9, ST_FLAG_IMMUTABLE);
+
+		/* note these are not immutable since we can reload
+		 * although the effect of this is questionable (FIXME)
+		 */
 		dparam_setinfo(var, val);
 		return;
 	}

server/netget.c

@@ -157,6 +157,11 @@ static void get_type(nut_ctype_t *client, const char *upsname, const char *var)
 	/* Any variable that is not string | range | enum is just a simple
 	 * numeric value */
 
+	if (!(node->flags & ST_FLAG_NUMBER)) {
+		upsdebugx(3, "%s: assuming that UPS[%s] variable %s which has no type flag is a NUMBER",
+			__func__, upsname, var);
+	}
+
 	sendback(client, "%s NUMBER\n", buf);
 }
 

server/netset.c

@@ -56,12 +56,19 @@ static void set_var(nut_ctype_t *client, const char *upsname, const char *var,
 	val = sstate_getinfo(ups, var);
 
 	if (!val) {
+		/* Technically, this includes state tree entries
+		 * with a defined name but null value so far.
+		 * FIXME? Use sstate_getnode() to differentiate
+		 *  the cases? Any practical use for this?
+		 */
 		send_err(client, NUT_ERR_VAR_NOT_SUPPORTED);
 		return;
 	}
 
 	/* make sure this variable is writable (RW) */
 	if ((sstate_getflags(ups, var) & ST_FLAG_RW) == 0) {
+		upsdebugx(3, "%s: UPS [%s]: value of %s is read-only",
+			__func__, ups->name, var);
 		send_err(client, NUT_ERR_READONLY);
 		return;
 	}
@@ -87,6 +94,8 @@ static void set_var(nut_ctype_t *client, const char *upsname, const char *var,
 		 * even on architectures with a moderate INTMAX
 		 */
 		if (aux < (int) strlen(newval)) {
+			upsdebugx(3, "%s: UPS [%s]: Not a value fitting in STRING:%ld %s: %s",
+				__func__, ups->name, aux, var, newval);
 			send_err(client, NUT_ERR_TOO_LONG);
 			return;
 		}
@@ -109,6 +118,9 @@ static void set_var(nut_ctype_t *client, const char *upsname, const char *var,
 		}
 
 		if (!found) {
+			/* Not a value known in the pre-defined enumeration */
+			upsdebugx(3, "%s: UPS [%s]: Not a value known in ENUM %s: %s",
+				__func__, ups->name, var, newval);
 			send_err(client, NUT_ERR_INVALID_VALUE);
 			return;
 		}
@@ -132,6 +144,9 @@ static void set_var(nut_ctype_t *client, const char *upsname, const char *var,
 		}
 
 		if (!found) {
+			/* Not in range */
+			upsdebugx(3, "%s: UPS [%s]: Not a value fitting in RANGE %s: %s",
+				__func__, ups->name, var, newval);
 			send_err(client, NUT_ERR_INVALID_VALUE);
 			return;
 		}