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;
 }