Replace strcmp() and strcasecmp() use-cases for short fixed string ar…

…gs by strncmp() and strncasecmp() respectively

Some compilers on some OSes complain due to implem nuances.
Updated docs/developers.txt to make note for posterity.

clients/upsclient.c

@@ -1418,8 +1418,8 @@ int upscli_list_next(UPSCONN_t *ups, unsigned int numq, const char **query,
 
 	/* see if this is the end */
 	if (ups->pc_ctx.numargs >= 2) {
-		if ((!strcmp(ups->pc_ctx.arglist[0], "END")) &&
-			(!strcmp(ups->pc_ctx.arglist[1], "LIST")))
+		if ((!strncmp(ups->pc_ctx.arglist[0], "END", 3)) &&
+			(!strncmp(ups->pc_ctx.arglist[1], "LIST", 4)))
 			return 0;
 	}
 

clients/upslog.c

@@ -488,7 +488,7 @@ int main(int argc, char **argv)
 		fprintf(stderr, "Warning: initial connect failed: %s\n",
 			upscli_strerror(&ups));
 
-	if (strcmp(logfn, "-") == 0)
+	if (strncmp(logfn, "-", 1) == 0)
 		logfile = stdout;
 	else
 		logfile = fopen(logfn, "a");

clients/upsmon.c

@@ -1591,19 +1591,19 @@ static void parse_status(utype_t *ups, char *status)
 
 		upsdebugx(3, "parsing: [%s]", statword);
 
-		if (!strcasecmp(statword, "OL"))
+		if (!strncasecmp(statword, "OL", 2))
 			ups_on_line(ups);
-		if (!strcasecmp(statword, "OB"))
+		if (!strncasecmp(statword, "OB", 2))
 			ups_on_batt(ups);
-		if (!strcasecmp(statword, "LB"))
+		if (!strncasecmp(statword, "LB", 2))
 			ups_low_batt(ups);
-		if (!strcasecmp(statword, "RB"))
+		if (!strncasecmp(statword, "RB", 2))
 			upsreplbatt(ups);
-		if (!strcasecmp(statword, "CAL"))
+		if (!strncasecmp(statword, "CAL", 3))
 			ups_cal(ups);
 
 		/* do it last to override any possible OL */
-		if (!strcasecmp(statword, "FSD"))
+		if (!strncasecmp(statword, "FSD", 3))
 			ups_fsd(ups);
 
 		update_crittimer(ups);

clients/upsrw.c

@@ -523,7 +523,7 @@ static void do_type(const char *varname)
 		}
 
 		/* ignore this one */
-		if (!strcasecmp(answer[i], "RW")) {
+		if (!strncasecmp(answer[i], "RW", 2)) {
 			continue;
 		}
 

clients/upssched.c

@@ -802,7 +802,7 @@ static void parse_at(const char *ntype, const char *un, const char *cmd,
 
 	/* check upsname: does this apply to us? */
 	if (strcmp(upsname, un) != 0)
-		if (strcmp(un, "*") != 0)
+		if (strncmp(un, "*", 1) != 0)
 			return;		/* not for us, and not the wildcard */
 
 	/* see if the current notify type matches the one from the .conf */
@@ -864,7 +864,7 @@ static int conf_arg(size_t numargs, char **arg)
 		return 0;
 
 	/* AT <notifytype> <upsname> <command> <cmdarg1> [<cmdarg2>] */
-	if (!strcmp(arg[0], "AT")) {
+	if (!strncmp(arg[0], "AT", 2)) {
 
 		/* don't use arg[5] unless we have it... */
 		if (numargs > 5)

clients/upsset.c

@@ -759,7 +759,7 @@ static void do_type(const char *varname)
 		}
 
 		/* ignore this one */
-		if (!strcasecmp(answer[i], "RW"))
+		if (!strncasecmp(answer[i], "RW", 2))
 			continue;
 
 		printf("Unrecognized\n");

common/state.c

@@ -422,7 +422,7 @@ void state_setflags(st_tree_t *root, const char *var, size_t numflags, char **fl
 
 	for (i = 0; i < numflags; i++) {
 
-		if (!strcasecmp(flag[i], "RW")) {
+		if (!strncasecmp(flag[i], "RW", 2)) {
 			sttmp->flags |= ST_FLAG_RW;
 			continue;
 		}

common/upsconf.c

@@ -54,7 +54,7 @@ static void conf_args(size_t numargs, char **arg)
 		return;
 
 	/* handle 'foo = bar', 'foo=bar', 'foo =bar' or 'foo= bar' forms */
-	if (!strcmp(arg[1], "=")) {
+	if (!strncmp(arg[1], "=", 1)) {
 		do_upsconf_args(ups_section, arg[0], arg[2]);
 		return;
 	}

docs/developers.txt

@@ -21,6 +21,13 @@ Don't use `strcat()`.  We have a neat wrapper for `snprintf()` called
 `snprintfcat()` that allows you to append to `char *` with a format
 string and all the usual string length checking of `snprintf()` routine.
 
+Some systems define `strcmp()` and `strcasecmp()` as macros or inline
+functions optimized for aligned memory access in a way that some compilers
+complain about array out-of-bounds access when comparing fixed short strings.
+For this reason, we compare strings with length 3 chars and less ("", "x",
+"on", "VAL") with fixed-length methods `strncmp()` and `strncasecmp()`,
+e.g. `strncmp(buf, "VAL", 3)`.
+
 Error reporting
 ~~~~~~~~~~~~~~~
 

docs/nut.dict

@@ -1,4 +1,4 @@
-personal_ws-1.1 en 2775 utf-8
+personal_ws-1.1 en 2792 utf-8
 AAS
 ACFAIL
 ACFREQ
@@ -2490,11 +2490,14 @@ strarr
 strcasecmp
 strcat
 strchr
+strcmp
 strcpy
 strdup
 strerror
 strftime
 strlen
+strncasecmp
+strncmp
 struct
 structs
 sts

drivers/apcsmart-old.c

@@ -237,7 +237,7 @@ static int poll_data(apc_vartab_t *vt)
 	}
 
 	/* no longer supported by the hardware somehow */
-	if (!strcmp(tmp, "NA")) {
+	if (!strncmp(tmp, "NA", 2)) {
 		dstate_delinfo(vt->name);
 		return 1;
 	}
@@ -291,7 +291,7 @@ static int query_ups(const char *var, int first)
 
 	ser_comm_good();
 
-	if ((ret < 1) || (!strcmp(temp, "NA")))		/* not supported */
+	if ((ret < 1) || (!strncmp(temp, "NA", 2)))		/* not supported */
 		return 0;
 
 	vt->flags |= APC_PRESENT;
@@ -330,7 +330,7 @@ static void do_capabilities(void)
 	ret = ser_get_line(upsfd, temp, sizeof(temp), ENDCHAR,
 		MINIGNCHARS, SER_WAIT_SEC, SER_WAIT_USEC);
 
-	if ((ret < 1) || (!strcmp(temp, "NA"))) {
+	if ((ret < 1) || (!strncmp(temp, "NA", 2))) {
 
 		/* Early Smart-UPS, not as smart as later ones */
 		/* This should never happen since we only call
@@ -434,7 +434,7 @@ static int update_status(void)
 
 	ret = read_buf(buf, sizeof(buf));
 
-	if ((ret < 1) || (!strcmp(buf, "NA"))) {
+	if ((ret < 1) || (!strncmp(buf, "NA", 2))) {
 		dstate_datastale();
 		return 0;
 	}
@@ -554,7 +554,7 @@ static int firmware_table_lookup(void)
 	 * Some UPSes support both 'V' and 'b'. As 'b' doesn't always return
 	 * firmware version, we attempt that only if 'V' doesn't work.
 	 */
-	if ((ret < 1) || (!strcmp(buf, "NA"))) {
+	if ((ret < 1) || (!strncmp(buf, "NA", 2))) {
 		upsdebugx(1, "Attempting firmware lookup using command 'b'");
 		ret = ser_send_char(upsfd, 'b');
 
@@ -633,7 +633,7 @@ static void getbaseinfo(void)
 	ret = ser_get_line(upsfd, temp, sizeof(temp), ENDCHAR, IGNCHARS,
 		SER_WAIT_SEC, SER_WAIT_USEC);
 
-	if ((ret < 1) || (!strcmp(temp, "NA"))) {
+	if ((ret < 1) || (!strncmp(temp, "NA", 2))) {
 		/* We have an old dumb UPS - go to specific code for old stuff */
 		oldapcsetup();
 		return;
@@ -684,7 +684,7 @@ static int do_cal(int start)
 	ret = read_buf(temp, sizeof(temp));
 
 	/* if we can't check the current calibration status, bail out */
-	if ((ret < 1) || (!strcmp(temp, "NA")))
+	if ((ret < 1) || (!strncmp(temp, "NA", 2)))
 		return STAT_INSTCMD_HANDLED;		/* FUTURE: failure */
 
 	tval = strtol(temp, 0, 16);
@@ -709,7 +709,7 @@ static int do_cal(int start)
 
 		ret = read_buf(temp, sizeof(temp));
 
-		if ((ret < 1) || (!strcmp(temp, "NA")) || (!strcmp(temp, "NO"))) {
+		if ((ret < 1) || (!strncmp(temp, "NA", 2)) || (!strncmp(temp, "NO", 2))) {
 			upslogx(LOG_WARNING, "Stop calibration failed: %s",
 				temp);
 			return STAT_INSTCMD_HANDLED;	/* FUTURE: failure */
@@ -736,7 +736,7 @@ static int do_cal(int start)
 
 	ret = read_buf(temp, sizeof(temp));
 
-	if ((ret < 1) || (!strcmp(temp, "NA")) || (!strcmp(temp, "NO"))) {
+	if ((ret < 1) || (!strncmp(temp, "NA", 2)) || (!strncmp(temp, "NO", 2))) {
 		upslogx(LOG_WARNING, "Start calibration failed: %s", temp);
 		return STAT_INSTCMD_HANDLED;	/* FUTURE: failure */
 	}
@@ -763,7 +763,7 @@ static int smartmode(void)
 			IGNCHARS, SER_WAIT_SEC, SER_WAIT_USEC);
 
 		if (ret > 0)
-			if (!strcmp(temp, "SM"))
+			if (!strncmp(temp, "SM", 2))
 				return 1;	/* success */
 
 		sleep(1);	/* wait before trying again */
@@ -795,7 +795,7 @@ static int sdok(void)
 	ser_get_line(upsfd, temp, sizeof(temp), ENDCHAR, IGNCHARS, SER_WAIT_SEC, SER_WAIT_USEC);
 	upsdebugx(4, "sdok: got \"%s\"", temp);
 
-	if (!strcmp(temp, "*") || !strcmp(temp, "OK")) {
+	if (!strncmp(temp, "*", 1) || !strncmp(temp, "OK", 2)) {
 		upsdebugx(4, "Last issued shutdown command succeeded");
 		return 1;
 	}
@@ -1057,7 +1057,7 @@ void upsdrv_shutdown(void)
 		status = APC_STAT_LB | APC_STAT_OB;
 	}
 
-	if (testvar("advorder") && strcasecmp(getval("advorder"), "no"))
+	if (testvar("advorder") && strncasecmp(getval("advorder"), "no", 2))
 		upsdrv_shutdown_advanced(status);
 	else
 		upsdrv_shutdown_simple(status);
@@ -1123,7 +1123,7 @@ static int setvar_enum(apc_vartab_t *vt, const char *val)
 
 	ret = read_buf(orig, sizeof(orig));
 
-	if ((ret < 1) || (!strcmp(orig, "NA")))
+	if ((ret < 1) || (!strncmp(orig, "NA", 2)))
 		return STAT_SET_HANDLED;	/* FUTURE: failed */
 
 	ptr = convert_data(vt, orig);
@@ -1147,13 +1147,13 @@ static int setvar_enum(apc_vartab_t *vt, const char *val)
 		/* this should return either OK (if rotated) or NO (if not) */
 		ret = read_buf(temp, sizeof(temp));
 
-		if ((ret < 1) || (!strcmp(temp, "NA")))
+		if ((ret < 1) || (!strncmp(temp, "NA", 2)))
 			return STAT_SET_HANDLED;	/* FUTURE: failed */
 
 		/* sanity checks */
-		if (!strcmp(temp, "NO"))
+		if (!strncmp(temp, "NO", 2))
 			return STAT_SET_HANDLED;	/* FUTURE: failed */
-		if (strcmp(temp, "OK") != 0)
+		if (strncmp(temp, "OK", 2) != 0)
 			return STAT_SET_HANDLED;	/* FUTURE: failed */
 
 		/* see what it rotated onto */
@@ -1166,7 +1166,7 @@ static int setvar_enum(apc_vartab_t *vt, const char *val)
 
 		ret = read_buf(temp, sizeof(temp));
 
-		if ((ret < 1) || (!strcmp(temp, "NA")))
+		if ((ret < 1) || (!strncmp(temp, "NA", 2)))
 			return STAT_SET_HANDLED;	/* FUTURE: failed */
 
 		ptr = convert_data(vt, temp);
@@ -1217,7 +1217,7 @@ static int setvar_string(apc_vartab_t *vt, const char *val)
 
 	ret = read_buf(temp, sizeof(temp));
 
-	if ((ret < 1) || (!strcmp(temp, "NA")))
+	if ((ret < 1) || (!strncmp(temp, "NA", 2)))
 		return STAT_SET_HANDLED;	/* FUTURE: failed */
 
 	/* suppress redundant changes - easier on the eeprom */
@@ -1267,7 +1267,7 @@ static int setvar_string(apc_vartab_t *vt, const char *val)
 		return STAT_SET_HANDLED;	/* FUTURE: failed */
 	}
 
-	if (!strcmp(temp, "NO")) {
+	if (!strncmp(temp, "NO", 2)) {
 		upslogx(LOG_ERR, "setvar_string: got NO at final read");
 		return STAT_SET_HANDLED;	/* FUTURE: failed */
 	}
@@ -1335,7 +1335,7 @@ static int do_cmd(apc_cmdtab_t *ct)
 	if (ret < 1)
 		return STAT_INSTCMD_HANDLED;		/* FUTURE: failed */
 
-	if (strcmp(buf, "OK") != 0) {
+	if (strncmp(buf, "OK", 2) != 0) {
 		upslogx(LOG_WARNING, "Got [%s] after command [%s]",
 			buf, ct->name);