brctl: optional support for "show" cmd (by L. Gabriel Somlo <somlo AT cmu.edu>)

function                                             old     new   delta
brctl_main                                           739    1186    +447
if_indextoname                                         -     104    +104
static.keywords                                      827     841     +14
static.ops                                             -       7      +7
packed_usage                                       23978   23976      -2

5 files changed, 147 insertions, 84 deletions

diff --git a/include/libbb.h b/include/libbb.h
index 202574adb..1c54a32d5 100644
--- a/include/libbb.h
+++ b/include/libbb.h
@@ -1017,15 +1017,15 @@ extern int update_passwd(const char *filename, const char *username,
 int get_terminal_width_height(int fd, int *width, int *height);
 
 int ioctl_or_perror(int fd, int request, void *argp, const char *fmt,...) __attribute__ ((format (printf, 4, 5)));
-void ioctl_or_perror_and_die(int fd, int request, void *argp, const char *fmt,...) __attribute__ ((format (printf, 4, 5)));
+int ioctl_or_perror_and_die(int fd, int request, void *argp, const char *fmt,...) __attribute__ ((format (printf, 4, 5)));
 #if ENABLE_IOCTL_HEX2STR_ERROR
 int bb_ioctl_or_warn(int fd, int request, void *argp, const char *ioctl_name);
-void bb_xioctl(int fd, int request, void *argp, const char *ioctl_name);
+int bb_xioctl(int fd, int request, void *argp, const char *ioctl_name);
 #define ioctl_or_warn(fd,request,argp) bb_ioctl_or_warn(fd,request,argp,#request)
 #define xioctl(fd,request,argp)        bb_xioctl(fd,request,argp,#request)
 #else
 int bb_ioctl_or_warn(int fd, int request, void *argp);
-void bb_xioctl(int fd, int request, void *argp);
+int bb_xioctl(int fd, int request, void *argp);
 #define ioctl_or_warn(fd,request,argp) bb_ioctl_or_warn(fd,request,argp)
 #define xioctl(fd,request,argp)        bb_xioctl(fd,request,argp)
 #endif
diff --git a/include/usage.h b/include/usage.h
index 0849c6831..e007acdfa 100644
--- a/include/usage.h
+++ b/include/usage.h
@@ -138,6 +138,9 @@
 #define brctl_full_usage \
        "Manage ethernet bridges.\n" \
      "\nCommands:" \
+	USE_FEATURE_BRCTL_SHOW( \
+     "\n	show			Show a list of bridges" \
+	) \
      "\n	addbr BRIDGE		Create BRIDGE" \
      "\n	delbr BRIDGE		Delete BRIDGE" \
      "\n	addif BRIDGE IFACE	Add IFACE to BRIDGE" \
@@ -151,7 +154,8 @@
      "\n	setportprio BRIDGE PRIO		Set port priority" \
      "\n	setbridgeprio BRIDGE PRIO	Set bridge priority" \
      "\n	stp BRIDGE [1|0]		STP on/off" \
-	)
+	) \
+
 #define bunzip2_trivial_usage \
        "[OPTION]... [FILE]"
 #define bunzip2_full_usage \
diff --git a/libbb/xfuncs.c b/libbb/xfuncs.c
index ca7f94173..125063935 100644
--- a/libbb/xfuncs.c
+++ b/libbb/xfuncs.c
@@ -704,17 +704,20 @@ int get_terminal_width_height(int fd, int *width, int *height)
 	return ret;
 }
 
-void ioctl_or_perror_and_die(int fd, int request, void *argp, const char *fmt,...)
+int ioctl_or_perror_and_die(int fd, int request, void *argp, const char *fmt,...)
 {
+	int ret;
 	va_list p;
 
-	if (ioctl(fd, request, argp) < 0) {
+	ret = ioctl(fd, request, argp);
+	if (ret < 0) {
 		va_start(p, fmt);
 		bb_verror_msg(fmt, p, strerror(errno));
 		/* xfunc_die can actually longjmp, so be nice */
 		va_end(p);
 		xfunc_die();
 	}
+	return ret;
 }
 
 int ioctl_or_perror(int fd, int request, void *argp, const char *fmt,...)
@@ -740,10 +743,14 @@ int bb_ioctl_or_warn(int fd, int request, void *argp, const char *ioctl_name)
 		bb_simple_perror_msg(ioctl_name);
 	return ret;
 }
-void bb_xioctl(int fd, int request, void *argp, const char *ioctl_name)
+int bb_xioctl(int fd, int request, void *argp, const char *ioctl_name)
 {
-	if (ioctl(fd, request, argp) < 0)
+	int ret;
+
+	ret = ioctl(fd, request, argp);
+	if (ret < 0)
 		bb_simple_perror_msg_and_die(ioctl_name);
+	return ret;
 }
 #else
 int bb_ioctl_or_warn(int fd, int request, void *argp)
@@ -755,9 +762,13 @@ int bb_ioctl_or_warn(int fd, int request, void *argp)
 		bb_perror_msg("ioctl %#x failed", request);
 	return ret;
 }
-void bb_xioctl(int fd, int request, void *argp)
+int bb_xioctl(int fd, int request, void *argp)
 {
-	if (ioctl(fd, request, argp) < 0)
+	int ret;
+
+	ret = ioctl(fd, request, argp);
+	if (ret < 0)
 		bb_perror_msg_and_die("ioctl %#x failed", request);
+	return ret;
 }
 #endif
diff --git a/networking/Config.in b/networking/Config.in
index f0a93078e..f7f189cd6 100644
--- a/networking/Config.in
+++ b/networking/Config.in
@@ -54,14 +54,6 @@ config BRCTL
 	  Manage ethernet bridges.
 	  Supports addbr/delbr and addif/delif.
 
-#config FEATURE_BRCTL_SHOW
-#	bool "Support show, showmac and showstp"
-#	default n
-#	depends on BRCTL
-#	help
-#	  Add support for option which print the current config:
-#	    showmacs, showstp, show
-
 config FEATURE_BRCTL_FANCY
 	bool "Fancy options"
 	default n
@@ -73,6 +65,14 @@ config FEATURE_BRCTL_FANCY
 	    stp
 	  This adds about 600 bytes.
 
+config FEATURE_BRCTL_SHOW
+	bool "Support show, showmac and showstp"
+	default n
+	depends on BRCTL && FEATURE_BRCTL_FANCY
+	help
+	  Add support for option which prints the current config:
+	    showmacs, showstp, show
+
 config DNSD
 	bool "dnsd"
 	default n
diff --git a/networking/brctl.c b/networking/brctl.c
index 2bb03dd82..bd13878b2 100644
--- a/networking/brctl.c
+++ b/networking/brctl.c
@@ -25,12 +25,6 @@
 /* #define BRCTL_USE_INTERNAL 0 */ /* use exact conversion */
 #define BRCTL_USE_INTERNAL 1
 
-#ifdef ENABLE_FEATURE_BRCTL_SHOW
-#error Remove these
-#endif
-#define ENABLE_FEATURE_BRCTL_SHOW 0
-#define USE_FEATURE_BRCTL_SHOW(...)
-
 #if ENABLE_FEATURE_BRCTL_FANCY
 #include <linux/if_bridge.h>
 
@@ -96,7 +90,7 @@ int brctl_main(int argc ATTRIBUTE_UNUSED, char **argv)
 		"setageing\0" "setfd\0" "sethello\0" "setmaxage\0"
 		"setpathcost\0" "setportprio\0" "setbridgeprio\0"
 	)
-		USE_FEATURE_BRCTL_SHOW("showmacs\0" "show\0");
+	USE_FEATURE_BRCTL_SHOW("showmacs\0" "show\0");
 
 	enum { ARG_addbr = 0, ARG_delbr, ARG_addif, ARG_delif
 		USE_FEATURE_BRCTL_FANCY(,
@@ -104,31 +98,87 @@ int brctl_main(int argc ATTRIBUTE_UNUSED, char **argv)
 		   ARG_setageing, ARG_setfd, ARG_sethello, ARG_setmaxage,
 		   ARG_setpathcost, ARG_setportprio, ARG_setbridgeprio
 		)
-		  USE_FEATURE_BRCTL_SHOW(, ARG_showmacs, ARG_show)
+		USE_FEATURE_BRCTL_SHOW(, ARG_showmacs, ARG_show)
 	};
 
 	int fd;
 	smallint key;
 	struct ifreq ifr;
 	char *br, *brif;
-#if ENABLE_FEATURE_BRCTL_FANCY
-	unsigned long args[4] = {0, 0, 0, 0};
-	int port;
-	int tmp;
-#endif
 
 	argv++;
 	while (*argv) {
+#if ENABLE_FEATURE_BRCTL_FANCY
+		int ifidx[MAX_PORTS];
+		unsigned long args[4];
+		ifr.ifr_data = (char *) &args;
+#endif
+
 		key = index_in_strings(keywords, *argv);
 		if (key == -1) /* no match found in keywords array, bail out. */
 			bb_error_msg_and_die(bb_msg_invalid_arg, *argv, applet_name);
 		argv++;
+		fd = xsocket(AF_INET, SOCK_STREAM, 0);
+
 #if ENABLE_FEATURE_BRCTL_SHOW
 		if (key == ARG_show) { /* show */
-			goto out; /* FIXME: implement me! :) */
+			char brname[IFNAMSIZ];
+			int bridx[MAX_PORTS];
+			int i, num;
+			arm_ioctl(args, BRCTL_GET_BRIDGES,
+						(unsigned long) bridx, MAX_PORTS);
+			num = xioctl(fd, SIOCGIFBR, args);
+			printf("bridge name\tbridge id\t\tSTP enabled\tinterfaces\n");
+			for (i = 0; i < num; i++) {
+				char ifname[IFNAMSIZ];
+				int j, tabs;
+				struct __bridge_info bi;
+				unsigned char *x;
+
+				if (!if_indextoname(bridx[i], brname))
+					bb_perror_msg_and_die("can't get bridge name for index %d", i);
+				safe_strncpy(ifr.ifr_name, brname, IFNAMSIZ);
+
+				arm_ioctl(args, BRCTL_GET_BRIDGE_INFO,
+							(unsigned long) &bi, 0);
+				xioctl(fd, SIOCDEVPRIVATE, &ifr);
+				printf("%s\t\t", brname);
+
+				/* print bridge id */
+				x = (unsigned char *) &bi.bridge_id;
+				for (j = 0; j < 8; j++) {
+					printf("%.2x", x[j]);
+					if (j == 1)
+						bb_putchar('.');
+				}
+				printf(bi.stp_enabled ? "\tyes" : "\tno");
+
+				/* print interface list */
+				arm_ioctl(args, BRCTL_GET_PORT_LIST,
+							(unsigned long) ifidx, MAX_PORTS);
+				xioctl(fd, SIOCDEVPRIVATE, &ifr);
+				tabs = 0;
+				for (j = 0; j < MAX_PORTS; j++) {
+					if (!ifidx[j])
+						continue;
+					if (!if_indextoname(ifidx[j], ifname))
+						bb_perror_msg_and_die("can't get interface name for index %d", j);
+					if (tabs)
+						printf("\t\t\t\t\t");
+					else
+						tabs = 1;
+					printf("\t\t%s\n", ifname);
+				}
+				if (!tabs)	/* bridge has no interfaces */
+					bb_putchar('\n');
+			}
+			goto done;
 		}
 #endif
-		fd = xsocket(AF_INET, SOCK_STREAM, 0);
+
+		if (!*argv) /* all but 'show' need at least one argument */
+			bb_show_usage();
+
 		br = *argv++;
 
 		if (key == ARG_addbr || key == ARG_delbr) { /* addbr or delbr */
@@ -137,11 +187,13 @@ int brctl_main(int argc ATTRIBUTE_UNUSED, char **argv)
 					br, "bridge %s", br);
 			goto done;
 		}
-		if (!*argv) /* all but 'show' need at least one argument */
+
+		if (!*argv) /* all but 'addif/delif' need at least two arguments */
 			bb_show_usage();
+
 		safe_strncpy(ifr.ifr_name, br, IFNAMSIZ);
 		if (key == ARG_addif || key == ARG_delif) { /* addif or delif */
-			brif = *argv++;
+			brif = *argv;
 			ifr.ifr_ifindex = if_nametoindex(brif);
 			if (!ifr.ifr_ifindex) {
 				bb_perror_msg_and_die("iface %s", brif);
@@ -149,76 +201,72 @@ int brctl_main(int argc ATTRIBUTE_UNUSED, char **argv)
 			ioctl_or_perror_and_die(fd,
 					key == ARG_addif ? SIOCBRADDIF : SIOCBRDELIF,
 					&ifr, "bridge %s", br);
-			goto done;
+			goto done_next_argv;
 		}
 #if ENABLE_FEATURE_BRCTL_FANCY
-		ifr.ifr_data = (char *) &args;
 		if (key == ARG_stp) { /* stp */
 			/* FIXME: parsing yes/y/on/1 versus no/n/off/0 is too involved */
 			arm_ioctl(args, BRCTL_SET_BRIDGE_STP_STATE,
 					  (unsigned)(**argv - '0'), 0);
 			goto fire;
 		}
-		if ((unsigned)(key - ARG_stp) < 5) { /* time related ops */
-			unsigned long op = (key == ARG_setageing) ? BRCTL_SET_AGEING_TIME :
-			                   (key == ARG_setfd) ? BRCTL_SET_BRIDGE_FORWARD_DELAY :
-			                   (key == ARG_sethello) ? BRCTL_SET_BRIDGE_HELLO_TIME :
-			                   /*key == ARG_setmaxage*/ BRCTL_SET_BRIDGE_MAX_AGE;
-			arm_ioctl(args, op, str_to_jiffies(*argv), 0);
+		if ((unsigned)(key - ARG_setageing) < 4) { /* time related ops */
+			static const uint8_t ops[] ALIGN1 = {
+				BRCTL_SET_AGEING_TIME,          /* ARG_setageing */
+				BRCTL_SET_BRIDGE_FORWARD_DELAY, /* ARG_setfd     */
+				BRCTL_SET_BRIDGE_HELLO_TIME,    /* ARG_sethello  */
+				BRCTL_SET_BRIDGE_MAX_AGE        /* ARG_setmaxage */
+			};
+			arm_ioctl(args, ops[key - ARG_setageing], str_to_jiffies(*argv), 0);
 			goto fire;
 		}
-		port = -1;
-		if (key == ARG_setpathcost || key == ARG_setportprio) {/* get portnum */
-			int ifidx[MAX_PORTS];
-			unsigned i;
-
-			port = if_nametoindex(*argv);
-			if (!port)
-				bb_error_msg_and_die(bb_msg_invalid_arg, *argv, "port");
-			argv++;
-			memset(ifidx, 0, sizeof ifidx);
-			arm_ioctl(args, BRCTL_GET_PORT_LIST, (unsigned long)ifidx,
-					  MAX_PORTS);
-			xioctl(fd, SIOCDEVPRIVATE, &ifr);
-			for (i = 0; i < MAX_PORTS; i++) {
-				if (ifidx[i] == port) {
-					port = i;
-					break;
-				}
-			}
-		}
 		if (key == ARG_setpathcost
 		 || key == ARG_setportprio
 		 || key == ARG_setbridgeprio
 		) {
-			unsigned long op = (key == ARG_setpathcost) ? BRCTL_SET_PATH_COST :
-			                   (key == ARG_setportprio) ? BRCTL_SET_PORT_PRIORITY :
-			                   /*key == ARG_setbridgeprio*/ BRCTL_SET_BRIDGE_PRIORITY;
-			unsigned long arg1 = port;
-			unsigned long arg2;
-# if BRCTL_USE_INTERNAL
-			tmp = xatoi(*argv);
-# else
-			if (sscanf(*argv, "%i", &tmp) != 1)
-				bb_error_msg_and_die(bb_msg_invalid_arg, *argv,
-						key == ARG_setpathcost ? "cost" : "prio");
-# endif
+			static const uint8_t ops[] ALIGN1 = {
+				BRCTL_SET_PATH_COST,      /* ARG_setpathcost   */
+				BRCTL_SET_PORT_PRIORITY,  /* ARG_setportprio   */
+				BRCTL_SET_BRIDGE_PRIORITY /* ARG_setbridgeprio */
+			};
+			int port = -1;
+			unsigned arg1, arg2;
+
+			if (key != ARG_setbridgeprio) {
+				/* get portnum */
+				unsigned i;
+
+				port = if_nametoindex(*argv++);
+				if (!port)
+					bb_error_msg_and_die(bb_msg_invalid_arg, *argv, "port");
+				memset(ifidx, 0, sizeof ifidx);
+				arm_ioctl(args, BRCTL_GET_PORT_LIST, (unsigned long)ifidx,
+						  MAX_PORTS);
+				xioctl(fd, SIOCDEVPRIVATE, &ifr);
+				for (i = 0; i < MAX_PORTS; i++) {
+					if (ifidx[i] == port) {
+						port = i;
+						break;
+					}
+				}
+			}
+			arg1 = port;
+			arg2 = xatoi_u(*argv);
 			if (key == ARG_setbridgeprio) {
-				arg1 = tmp;
+				arg1 = arg2;
 				arg2 = 0;
-			} else
-				arg2 = tmp;
-			arm_ioctl(args, op, arg1, arg2);
+			}
+			arm_ioctl(args, ops[key - ARG_setpathcost], arg1, arg2);
 		}
  fire:
-		/* Execute the previously set command.  */
+		/* Execute the previously set command */
 		xioctl(fd, SIOCDEVPRIVATE, &ifr);
-		argv++;
 #endif
+ done_next_argv:
+		argv++;
  done:
-		if (ENABLE_FEATURE_CLEAN_UP)
-			close(fd);
+		close(fd);
 	}
- USE_FEATURE_BRCTL_SHOW(out:)
+
 	return EXIT_SUCCESS;
 }