Revert "Allow changing the region without uploading a firmware file"

This reverts commit 6838a3a546.

ethsock.c

@@ -1325,8 +1325,6 @@ static int ethsock_ip_add_del(struct ethsock *sock, uint32_t ipaddr, uint32_t ip
 
 		(*undo)->ip[0] = ipaddr;
 		(*undo)->ip[1] = ipmask;
-	} else if (!add && (!undo || !*undo)) {
-		return 0;
 	}
 
 	ret = -1;

main.c

@@ -29,7 +29,7 @@ void usage(FILE *fp)
 	fprintf(fp,
 			"Usage: nmrpflash [OPTIONS...]\n"
 			"\n"
-			"Options (-i, and either -f, -c, or -R are mandatory):\n"
+			"Options (-i, and -f or -c are mandatory):\n"
 			" -a <ipaddr>     IP address to assign to target device [%s]\n"
 			" -A <ipaddr>     IP address to assign to selected interface [%s]\n"
 			" -B              Blind mode (don't wait for response packets)\n"
@@ -42,7 +42,9 @@ void usage(FILE *fp)
 			" -t <timeout>    Timeout (in milliseconds) for NMRP packets [%d ms]\n"
 			" -T <timeout>    Time (seconds) to wait after successfull TFTP upload [%d s]\n"
 			" -p <port>       Port to use for TFTP upload [%d]\n"
-			" -R <region>     Set device region (BZ, GR, IN, JP, KO, NA, PR, RU, WW)\n"
+#ifdef NMRPFLASH_SET_REGION
+			" -R <region>     Set device region (NA, WW, GR, PR, RU, BZ, IN, KO, JP)\n"
+#endif
 			" -S <n>          Skip <n> bytes of the firmware file\n"
 #ifdef NMRPFLASH_TFTP_TEST
 			" -U              Test TFTP upload\n"
@@ -61,19 +63,15 @@ void usage(FILE *fp)
 			")\n\n"
 #ifndef NMRPFLASH_WINDOWS
 			"# nmrpflash -i eth0 -f firmware.bin\n"
-			"\nor\n\n"
-			"# nmrpflash -i eth0 -R WW\n"
 #else
 			"C:\\> nmrpflash.exe -i net0 -f firmware.bin\n"
-			"\nor\n\n"
-			"C:\\> nmrpflash.exe -i net0 -R WW\n"
 #endif
 			"\n"
 			"When using -c, the environment variables IP, PORT, NETMASK\n"
 			"and MAC are set to the device IP address, TFTP port, subnet\n"
 			"mask and MAC address, respectively.\n"
 			"\n"
-			"nmrpflash %s, Copyright (C) 2016-2024 Joseph C. Lehner\n"
+			"nmrpflash %s, Copyright (C) 2016-2023 Joseph C. Lehner\n"
 			"nmrpflash is free software, licensed under the GNU GPLv3.\n"
 			"Source code at https://github.com/jclehner/nmrpflash\n"
 			"\n"
@@ -153,7 +151,7 @@ int main(int argc, char **argv)
 		.ipmask = NMRP_DEFAULT_SUBNET,
 		.intf = NULL,
 		.mac = "ff:ff:ff:ff:ff:ff",
-		.op = -1,
+		.op = NMRP_UPLOAD_FW,
 		.port = NMRP_DEFAULT_TFTP_PORT,
 		.region = NULL,
 		.blind = false,
@@ -229,9 +227,11 @@ int main(int argc, char **argv)
 			case 'M':
 				args.ipmask = optarg;
 				break;
+#ifdef NMRPFLASH_SET_REGION
 			case 'R':
 				args.region = optarg;
 				break;
+#endif
 			case 'p':
 			case 'S':
 			case 'T':
@@ -297,17 +297,9 @@ int main(int argc, char **argv)
 	}
 
 #ifndef NMRPFLASH_FUZZ
-	if (!list) {
+	if (!list && ((!args.file_local && !args.tftpcmd) || !args.intf)) {
-		if (args.file_local || args.tftpcmd) {
+		usage(stderr);
-			args.op = NMRP_UPLOAD_FW;
+		return 1;
-		} else if (args.region) {
-			args.op = NMRP_SET_REGION;
-		}
-
-		if (args.op == -1 || !args.intf) {
-			usage(stderr);
-			return 1;
-		}
 	}
 
 	if (!list) {

nmrp.c

@@ -212,21 +212,20 @@ static void msg_mkadvertise(struct nmrp_msg *msg, const char *magic)
 	msg_mkopt(msg, msg->opts, NMRP_O_MAGIC_NO, magic, strlen(magic));
 }
 
-static void msg_mkconfack(struct nmrp_msg *msg, uint32_t ipaddr, uint32_t ipmask, uint16_t region, enum nmrp_op op)
+static void msg_mkconfack(struct nmrp_msg *msg, uint32_t ipaddr, uint32_t ipmask, uint16_t region)
 {
-	char *p = msg->opts;
+	char *p;
 	uint32_t ip[2] = { ipaddr, ipmask };
 
 	msg_init(msg, NMRP_C_CONF_ACK);
+	p = msg_mkopt(msg, msg->opts, NMRP_O_DEV_IP, &ip, 8);
+	p = msg_mkopt(msg, p, NMRP_O_FW_UP, NULL, 0);
 
-	if (op != NMRP_SET_REGION) {
+#ifdef NMRPFLASH_SET_REGION
-		p = msg_mkopt(msg, p, NMRP_O_DEV_IP, &ip, 8);
-		p = msg_mkopt(msg, p, NMRP_O_FW_UP, NULL, 0);
-	}
-
 	if (region) {
 		p = msg_mkopt(msg, p, NMRP_O_DEV_REGION, &region, 2);
 	}
+#endif
 }
 
 #ifdef NMRPFLASH_FUZZ
@@ -397,54 +396,57 @@ int nmrp_do(struct nmrpd_args *args)
 	struct in_addr ipaddr;
 	struct in_addr ipmask;
 
-	if (args->op == NMRP_UPLOAD_FW) {
+	if (args->op != NMRP_UPLOAD_FW) {
-		if (!mac_parse(args->mac, dest)) {
+		fprintf(stderr, "Operation not implemented.\n");
-			fprintf(stderr, "Invalid MAC address '%s'.\n", args->mac);
+		return 1;
+	}
+
+	if (!mac_parse(args->mac, dest)) {
+		fprintf(stderr, "Invalid MAC address '%s'.\n", args->mac);
+		return 1;
+	}
+
+	ipmask.s_addr = inet_addr(args->ipmask);
+	if (ipmask.s_addr == INADDR_NONE
+			|| netmask(bitcount(ipmask.s_addr)) != ipmask.s_addr) {
+		fprintf(stderr, "Invalid subnet mask '%s'.\n", args->ipmask);
+		return 1;
+	}
+
+	if (!args->ipaddr) {
+		autoip = true;
+		args->ipaddr = NMRP_DEFAULT_IP_REMOTE;
+
+		if (!args->ipaddr_intf) {
+			args->ipaddr_intf = NMRP_DEFAULT_IP_LOCAL;
+		}
+	} else if (args->ipaddr_intf) {
+		autoip = true;
+	} else {
+		autoip = false;
+	}
+
+	if ((ipaddr.s_addr = inet_addr(args->ipaddr)) == INADDR_NONE) {
+		fprintf(stderr, "Invalid IP address '%s'.\n", args->ipaddr);
+		return 1;
+	}
+
+	if (args->ipaddr_intf && (intf_addr = inet_addr(args->ipaddr_intf)) == INADDR_NONE) {
+		fprintf(stderr, "Invalid IP address '%s'.\n", args->ipaddr_intf);
+		return 1;
+	}
+
+	if (args->file_local && strcmp(args->file_local, "-") && access(args->file_local, R_OK) == -1) {
+		fprintf(stderr, "Error accessing file '%s'.\n", args->file_local);
+		return 1;
+	}
+
+	if (args->file_remote) {
+		if (!tftp_is_valid_filename(args->file_remote)) {
+			fprintf(stderr, "Invalid remote filename '%s'.\n",
+					args->file_remote);
 			return 1;
 		}
-
-		ipmask.s_addr = inet_addr(args->ipmask);
-		if (ipmask.s_addr == INADDR_NONE
-				|| netmask(bitcount(ipmask.s_addr)) != ipmask.s_addr) {
-			fprintf(stderr, "Invalid subnet mask '%s'.\n", args->ipmask);
-			return 1;
-		}
-
-		if (!args->ipaddr) {
-			autoip = true;
-			args->ipaddr = NMRP_DEFAULT_IP_REMOTE;
-
-			if (!args->ipaddr_intf) {
-				args->ipaddr_intf = NMRP_DEFAULT_IP_LOCAL;
-			}
-		} else if (args->ipaddr_intf) {
-			autoip = true;
-		} else {
-			autoip = false;
-		}
-
-		if ((ipaddr.s_addr = inet_addr(args->ipaddr)) == INADDR_NONE) {
-			fprintf(stderr, "Invalid IP address '%s'.\n", args->ipaddr);
-			return 1;
-		}
-
-		if (args->ipaddr_intf && (intf_addr = inet_addr(args->ipaddr_intf)) == INADDR_NONE) {
-			fprintf(stderr, "Invalid IP address '%s'.\n", args->ipaddr_intf);
-			return 1;
-		}
-
-		if (args->file_local && strcmp(args->file_local, "-") && access(args->file_local, R_OK) == -1) {
-			fprintf(stderr, "Error accessing file '%s'.\n", args->file_local);
-			return 1;
-		}
-
-		if (args->file_remote) {
-			if (!tftp_is_valid_filename(args->file_remote)) {
-				fprintf(stderr, "Invalid remote filename '%s'.\n",
-						args->file_remote);
-				return 1;
-			}
-		}
 	}
 
 	if (args->region) {
@@ -457,10 +459,6 @@ int nmrp_do(struct nmrpd_args *args)
 		region = 0;
 	}
 
-	if (args->op == NMRP_SET_REGION && !region) {
-		fprintf(stderr, "No region code specified.\n");
-	}
-
 	status = 1;
 
 	sock = ethsock_create(args->intf, ETH_P_NMRP);
@@ -500,24 +498,22 @@ int nmrp_do(struct nmrpd_args *args)
 		}
 	}
 
-	if (args->op != NMRP_SET_REGION) {
+	if (!autoip) {
-		if (!autoip) {
+		status = is_valid_ip(sock, &ipaddr, &ipmask);
-			status = is_valid_ip(sock, &ipaddr, &ipmask);
+		if (status <= 0) {
-			if (status <= 0) {
+			if (!status) {
-				if (!status) {
+				fprintf(stderr, "Address %s/%s cannot be used on interface %s.\n",
-					fprintf(stderr, "Address %s/%s cannot be used on interface %s.\n",
+						args->ipaddr, args->ipmask, args->intf);
-							args->ipaddr, args->ipmask, args->intf);
-				}
-				goto out;
-			}
-		} else {
-			if (verbosity) {
-				printf("Adding %s to interface %s.\n", args->ipaddr_intf, args->intf);
 			}
+			goto out;
+		}
+	} else {
+		if (verbosity) {
+			printf("Adding %s to interface %s.\n", args->ipaddr_intf, args->intf);
+		}
 
-			if (ethsock_ip_add(sock, intf_addr, ipmask.s_addr, &ip_undo) != 0) {
+		if (ethsock_ip_add(sock, intf_addr, ipmask.s_addr, &ip_undo) != 0) {
-				goto out;
+			goto out;
-			}
 		}
 	}
 
@@ -616,28 +612,17 @@ int nmrp_do(struct nmrpd_args *args)
 				status = 1;
 				goto out;
 			case NMRP_C_CONF_REQ:
-				msg_mkconfack(&tx.msg, ipaddr.s_addr, ipmask.s_addr, region, args->op);
+				msg_mkconfack(&tx.msg, ipaddr.s_addr, ipmask.s_addr, region);
+				expect = NMRP_C_TFTP_UL_REQ;
 
 				if (!args->blind) {
 					printf("Received configuration request from %s.\n",
 							mac_to_str(rx.eh.ether_shost));
 				}
 
-				printf("Sending configuration:");
+				printf("Sending configuration: %s/%d.\n",
+						args->ipaddr, bitcount(ipmask.s_addr));
 
-				if (args->op == NMRP_SET_REGION) {
-					expect = NMRP_C_CLOSE_REQ;
-				} else {
-					printf(" %s/%d", args->ipaddr, bitcount(ipmask.s_addr));
-					// no support for NMRP_UPLOAD_ST
-					expect = NMRP_C_TFTP_UL_REQ;
-				}
-
-				if (region) {
-					printf(" region %s", args->region);
-				}
-
-				printf(".\n");
 				break;
 			case NMRP_C_TFTP_UL_REQ:
 				if (!upload_ok) {

nmrpd.h

@@ -70,6 +70,8 @@
 #define PACKED __attribute__((packed))
 #endif
 
+#define NMRPFLASH_SET_REGION
+
 #define NMRP_DEFAULT_UL_TIMEOUT_S    (30 * 60)
 #define NMRP_DEFAULT_RX_TIMEOUT_MS   (10000)
 /*