Fixes for 0.9.7

	- error checking
	- handle upto 1000's of bridges
	- -V version
	- don't initialize bridge till have to.

12 files changed, 221 insertions, 203 deletions

diff --git a/AUTHORS b/AUTHORS
index 14f6fc8..9880de1 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -1 +1,2 @@
+Stephen Hemminger <shemminger@osdl.org>
 Lennert Buytenhek <buytenh@gnu.org>
diff --git a/brctl/Makefile.in b/brctl/Makefile.in
index d238e5e..23515ef 100644
--- a/brctl/Makefile.in
+++ b/brctl/Makefile.in
@@ -25,7 +25,8 @@ brctld_OBJECTS= $(brctld_SOURCES:.c=.o)
 
 OBJECTS= $(common_OBJECTS) $(brctl_OBJECTS) $(brctld_OBJECTS)
 
-PROGRAMS= brctl brctld
+# brctld is for debug only, not production use
+PROGRAMS= brctl 
 
 
 all:	$(PROGRAMS)
diff --git a/brctl/brctl.c b/brctl/brctl.c
index f4d137a..1849373 100644
--- a/brctl/brctl.c
+++ b/brctl/brctl.c
@@ -19,12 +19,13 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-#include <sys/time.h>
-#include <asm/param.h>
+#include <sys/errno.h>
 #include "libbridge.h"
 #include "brctl.h"
 
-char *help_message =
+const char *version = "0.9.7";
+
+const char *help_message =
 "commands:\n"
 "\taddbr\t\t<bridge>\t\tadd bridge\n"
 "\taddif\t\t<bridge> <device>\tadd interface to bridge\n"
@@ -37,7 +38,6 @@ char *help_message =
 "\tsetageing\t<bridge> <time>\t\tset ageing time\n"
 "\tsetbridgeprio\t<bridge> <prio>\t\tset bridge priority\n"
 "\tsetfd\t\t<bridge> <time>\t\tset bridge forward delay\n"
-"\tsetgcint\t<bridge> <time>\t\tset garbage collection interval\n"
 "\tsethello\t<bridge> <time>\t\tset hello time\n"
 "\tsetmaxage\t<bridge> <time>\t\tset max message age\n"
 "\tsetpathcost\t<bridge> <port> <cost>\tset path cost\n"
@@ -55,11 +55,20 @@ int main(int argc, char *argv[])
 	struct bridge *br;
 	struct command *cmd;
 
-	br_init();
-
 	if (argc < 2)
 		goto help;
 
+	if (strcmp(argv[1], "-V") == 0) {
+		fprintf(stderr, "bridge-utils %s\n", version);
+		return 0;
+	}
+
+	if (br_init()) {
+		fprintf(stderr, "can't setup bridge control: %s\n",
+			strerror(errno));
+		return 1;
+	}
+
 	if ((cmd = br_command_lookup(argv[1])) == NULL) {
 		fprintf(stderr, "never heard of command [%s]\n", argv[1]);
 		goto help;
@@ -74,9 +83,9 @@ int main(int argc, char *argv[])
 		}
 
 		br = br_find_bridge(argv[argindex]);
-
+	
 		if (br == NULL) {
-			fprintf(stderr, "bridge %s doesn't exist!\n", argv[argindex]);
+			fprintf(stderr, "bridge %s doesn't exist?\n", argv[argindex]);
 			return 1;
 		}
 
diff --git a/brctl/brctl.h b/brctl/brctl.h
index c582f47..c8a6c0c 100644
--- a/brctl/brctl.h
+++ b/brctl/brctl.h
@@ -28,10 +28,10 @@ struct command
 };
 
 struct command *br_command_lookup(char *cmd);
-void br_dump_bridge_id(unsigned char *x);
-void br_show_timer(struct timeval *tv);
-void br_dump_interface_list(struct bridge *br);
-void br_dump_port_info(struct port *p);
-void br_dump_info(struct bridge *br);
+void br_dump_bridge_id(const unsigned char *x);
+void br_show_timer(const struct timeval *tv);
+void br_dump_interface_list(const struct bridge *br);
+void br_dump_port_info(const struct port *p);
+void br_dump_info(const struct bridge *br);
 
 #endif
diff --git a/brctl/brctl_cmd.c b/brctl/brctl_cmd.c
index b56b0c0..5a5e87d 100644
--- a/brctl/brctl_cmd.c
+++ b/brctl/brctl_cmd.c
@@ -25,6 +25,16 @@
 #include "libbridge.h"
 #include "brctl.h"
 
+static int strtotimeval(struct timeval *tv, const char *time)
+{
+	double secs;
+	if (sscanf(time, "%lf", &secs) != 1) 
+		return -1;
+	tv->tv_sec = secs;
+	tv->tv_usec = 1000000 * (secs - tv->tv_sec);
+	return 0;
+}
+
 void br_cmd_addbr(struct bridge *br, char *brname, char *arg1)
 {
 	int err;
@@ -39,7 +49,8 @@ void br_cmd_addbr(struct bridge *br, char *brname, char *arg1)
 		break;
 
 	default:
-		perror("br_add_bridge");
+		fprintf(stderr, "add bridge failed: %s\n",
+			strerror(err));
 		break;
 	}
 }
@@ -63,7 +74,8 @@ void br_cmd_delbr(struct bridge *br, char *brname, char *arg1)
 		break;
 
 	default:
-		perror("br_del_bridge");
+		fprintf(stderr, "can't delete bridge %s: %s\n",
+			brname, strerror(err));
 		break;
 	}
 }
@@ -96,7 +108,8 @@ void br_cmd_addif(struct bridge *br, char *ifname, char *arg1)
 		break;
 
 	default:
-		perror("br_add_interface");
+		fprintf(stderr, "can't add %s to bridge %s: %s\n",
+			ifname, br->ifname, strerror(err));
 		break;
 	}
 }
@@ -122,111 +135,149 @@ void br_cmd_delif(struct bridge *br, char *ifname, char *arg1)
 		break;
 
 	default:
-		perror("br_del_interface");
+		fprintf(stderr, "can't delete %s from %s: %s\n",
+			ifname, br->ifname, strerror(err));
 		break;
 	}
 }
 
 void br_cmd_setageing(struct bridge *br, char *time, char *arg1)
 {
-	double secs;
+	int err;
 	struct timeval tv;
 
-	sscanf(time, "%lf", &secs);
-	tv.tv_sec = secs;
-	tv.tv_usec = 1000000 * (secs - tv.tv_sec);
-	br_set_ageing_time(br, &tv);
+	if (strtotimeval(&tv, time)) {
+		fprintf(stderr, "bad ageing time value\n");
+		return;
+	}
+	err = br_set_ageing_time(br, &tv);
+	if (err)
+		fprintf(stderr, "set ageing time failed: %s\n",
+			strerror(err));
 }
 
 void br_cmd_setbridgeprio(struct bridge *br, char *_prio, char *arg1)
 {
 	int prio;
+	int err;
 
-	sscanf(_prio, "%i", &prio);
-	br_set_bridge_priority(br, prio);
+	if (sscanf(_prio, "%i", &prio) != 1) {
+		fprintf(stderr,"bad priority\n");
+		return;
+	}
+	err = br_set_bridge_priority(br, prio);
+	if (err)
+		fprintf(stderr, "set bridge priority failed: %s\n",
+			strerror(err));
 }
 
 void br_cmd_setfd(struct bridge *br, char *time, char *arg1)
 {
-	double secs;
 	struct timeval tv;
+	int err;
 
-	sscanf(time, "%lf", &secs);
-	tv.tv_sec = secs;
-	tv.tv_usec = 1000000 * (secs - tv.tv_sec);
-	br_set_bridge_forward_delay(br, &tv);
-}
-
-void br_cmd_setgcint(struct bridge *br, char *time, char *arg1)
-{
-	double secs;
-	struct timeval tv;
+	if (strtotimeval(&tv, time)) {
+		fprintf(stderr, "bad forward delay value\n");
+		return;
+	}
 
-	sscanf(time, "%lf", &secs);
-	tv.tv_sec = secs;
-	tv.tv_usec = 1000000 * (secs - tv.tv_sec);
-	br_set_gc_interval(br, &tv);
+	err = br_set_bridge_forward_delay(br, &tv);
+	if (err)
+		fprintf(stderr, "set forward delay failed: %s\n",
+			strerror(err));
 }
 
 void br_cmd_sethello(struct bridge *br, char *time, char *arg1)
 {
-	double secs;
 	struct timeval tv;
+	int err;
+
+	if (strtotimeval(&tv, time)) {
+		fprintf(stderr, "bad hello timer value\n");
+		return;
+	}
 
-	sscanf(time, "%lf", &secs);
-	tv.tv_sec = secs;
-	tv.tv_usec = 1000000 * (secs - tv.tv_sec);
-	br_set_bridge_hello_time(br, &tv);
+	err = br_set_bridge_hello_time(br, &tv);
+	if (err)
+		fprintf(stderr, "set hello timer failed: %s\n",
+			strerror(err));
 }
 
 void br_cmd_setmaxage(struct bridge *br, char *time, char *arg1)
 {
-	double secs;
 	struct timeval tv;
+	int err;
 
-	sscanf(time, "%lf", &secs);
-	tv.tv_sec = secs;
-	tv.tv_usec = 1000000 * (secs - tv.tv_sec);
-	br_set_bridge_max_age(br, &tv);
+	if (strtotimeval(&tv, time)) {
+		fprintf(stderr, "bad max age value\n");
+		return;
+	}
+	err = br_set_bridge_max_age(br, &tv);
+	if (err)
+		fprintf(stderr, "set max age failed: %s\n",
+			strerror(err));
 }
 
 void br_cmd_setpathcost(struct bridge *br, char *arg0, char *arg1)
 {
-	int cost;
+	int cost, err;
 	struct port *p;
 
+	if (sscanf(arg1, "%i", &cost) != 1) {
+		fprintf(stderr, "bad path cost value\n");
+		return;
+	}
+
 	if ((p = br_find_port(br, arg0)) == NULL) {
 		fprintf(stderr, "can't find port %s in bridge %s\n", arg0, br->ifname);
 		return;
 	}
 
-	sscanf(arg1, "%i", &cost);
-	br_set_path_cost(p, cost);
+	err = br_set_path_cost(p, cost);
+	if (err)
+		fprintf(stderr, "set path cost failed: %s\n",
+			strerror(err));
 }
 
 void br_cmd_setportprio(struct bridge *br, char *arg0, char *arg1)
 {
-	int cost;
+	int cost, err;
 	struct port *p;
 
+	if (sscanf(arg1, "%i", &cost) != 1) {
+		fprintf(stderr, "bad path priority value\n");
+		return;
+	}
+
 	if ((p = br_find_port(br, arg0)) == NULL) {
 		fprintf(stderr, "can't find port %s in bridge %s\n", arg0, br->ifname);
 		return;
 	}
-
-	sscanf(arg1, "%i", &cost);
-	br_set_port_priority(p, cost);
+	err = br_set_port_priority(p, cost);
+	if (err)
+		fprintf(stderr, "set port priority failed: %s\n",
+			strerror(err));
 }
 
 void br_cmd_stp(struct bridge *br, char *arg0, char *arg1)
 {
-	int stp;
+	int stp, err;
 
-	stp = 0;
-	if (!strcmp(arg0, "on") || !strcmp(arg0, "yes") || !strcmp(arg0, "1"))
+	if (!strcmp(arg0, "on") || !strcmp(arg0, "yes") 
+	    || !strcmp(arg0, "1"))
 		stp = 1;
+	else if (!strcmp(arg0, "off") || !strcmp(arg0, "no") 
+		 || !strcmp(arg0, "0"))
+		stp = 0;
+	else {
+		fprintf(stderr, "expect on/off for argument\n");
+		return;
+	}
 
-	br_set_stp_state(br, stp);
+	err = br_set_stp_state(br, stp);
+	if (err)
+		fprintf(stderr, "set stp status failed: %s\n",
+			strerror(err));
 }
 
 void br_cmd_showstp(struct bridge *br, char *arg0, char *arg1)
@@ -253,18 +304,10 @@ static int compare_fdbs(const void *_f0, const void *_f1)
 	const struct fdb_entry *f0 = _f0;
 	const struct fdb_entry *f1 = _f1;
 
-#if 0
-	if (f0->port_no < f1->port_no)
-		return -1;
-
-	if (f0->port_no > f1->port_no)
-		return 1;
-#endif
-
 	return memcmp(f0->mac_addr, f1->mac_addr, 6);
 }
 
-void __dump_fdb_entry(struct fdb_entry *f)
+static void __dump_fdb_entry(const struct fdb_entry *f)
 {
 	printf("%3i\t", f->port_no);
 	printf("%.2x:%.2x:%.2x:%.2x:%.2x:%.2x\t",
@@ -292,10 +335,10 @@ void br_cmd_showmacs(struct bridge *br, char *arg0, char *arg1)
 			fprintf(stderr, "read of forward table failed\n");
 			break;
 		}
-
+				
 		if (!num)
 			break;
-
+		
 		qsort(fdb, num, sizeof(struct fdb_entry), compare_fdbs);
 
 		for (i=0;i<num;i++)
@@ -313,7 +356,6 @@ static struct command commands[] = {
 	{1, 1, "setageing", br_cmd_setageing},
 	{1, 1, "setbridgeprio", br_cmd_setbridgeprio},
 	{1, 1, "setfd", br_cmd_setfd},
-	{1, 1, "setgcint", br_cmd_setgcint},
 	{1, 1, "sethello", br_cmd_sethello},
 	{1, 1, "setmaxage", br_cmd_setmaxage},
 	{1, 2, "setpathcost", br_cmd_setpathcost},
diff --git a/bridge-utils.spec b/bridge-utils.spec
index e0f54ae..d696027 100644
--- a/bridge-utils.spec
+++ b/bridge-utils.spec
@@ -1,6 +1,6 @@
 Summary: Utilities for configuring the linux ethernet bridge.
 Name: bridge-utils
-Version: 0.9.6
+Version: 0.9.7
 Release: 1
 Copyright: GPL
 Group: System Environment/Base
diff --git a/libbridge/libbridge.h b/libbridge/libbridge.h
index c704449..2074d80 100644
--- a/libbridge/libbridge.h
+++ b/libbridge/libbridge.h
@@ -64,7 +64,6 @@ struct bridge
 	int ifindex;
 	char ifname[IFNAMSIZ];
 	struct port *firstport;
-	struct port *ports[256];
 	struct bridge_info info;
 };
 
@@ -95,7 +94,6 @@ struct port_info
 struct port
 {
 	struct port *next;
-
 	int index;
 	int ifindex;
 	struct bridge *parent;
@@ -106,10 +104,11 @@ extern struct bridge *bridge_list;
 
 int br_init(void);
 int br_refresh(void);
-struct bridge *br_find_bridge(char *brname);
-struct port *br_find_port(struct bridge *br, char *portname);
-char *br_get_state_name(int state);
+struct bridge *br_find_bridge(const char *brname);
+struct port *br_find_port(struct bridge *br, const char *portname);
+const char *br_get_state_name(int state);
 
+int br_get_version(void);
 int br_add_bridge(const char *brname);
 int br_del_bridge(const char *brname);
 int br_add_interface(struct bridge *br, int ifindex);
diff --git a/libbridge/libbridge_devif.c b/libbridge/libbridge_devif.c
index b1189d3..acd8b00 100644
--- a/libbridge/libbridge_devif.c
+++ b/libbridge/libbridge_devif.c
@@ -26,7 +26,7 @@
 #include "libbridge.h"
 #include "libbridge_private.h"
 
-int br_device_ioctl32(struct bridge *br, unsigned long arg0, unsigned long arg1, unsigned long arg2, unsigned long arg3)
+int br_device_ioctl(struct bridge *br, unsigned long arg0, unsigned long arg1, unsigned long arg2, unsigned long arg3)
 {
 	unsigned long args[4];
 	struct ifreq ifr;
@@ -36,40 +36,12 @@ int br_device_ioctl32(struct bridge *br, unsigned long arg0, unsigned long arg1,
 	args[2] = arg2;
 	args[3] = arg3;
 
-	memcpy(ifr.ifr_name, br->ifname, IFNAMSIZ);
+	strncpy(ifr.ifr_name, br->ifname, IFNAMSIZ);
 	((unsigned long *)(&ifr.ifr_data))[0] = (unsigned long)args;
 
 	return ioctl(br_socket_fd, SIOCDEVPRIVATE, &ifr);
 }
 
-#ifdef __sparc__
-int br_device_ioctl64(struct bridge *br, unsigned long arg0, unsigned long arg1, unsigned long arg2, unsigned long arg3)
-{
-	unsigned long long args[4];
-	struct ifreq ifr;
-
-	args[0] = arg0;
-	args[1] = arg1;
-	args[2] = arg2;
-	args[3] = arg3;
-
-	memcpy(ifr.ifr_name, br->ifname, IFNAMSIZ);
-	((unsigned long long *)(&ifr.ifr_data))[0] = (unsigned long long)(unsigned long)args;
-
-	return ioctl(br_socket_fd, SIOCDEVPRIVATE + 3, &ifr);
-}
-#endif
-
-int br_device_ioctl(struct bridge *br, unsigned long arg0, unsigned long arg1, unsigned long arg2, unsigned long arg3)
-{
-#ifdef __sparc__
-	if (__kernel_is_64_bit())
-		return br_device_ioctl64(br, arg0, arg1, arg2, arg3);
-#endif
-
-	return br_device_ioctl32(br, arg0, arg1, arg2, arg3);
-}
-
 int br_add_interface(struct bridge *br, int ifindex)
 {
 	if (br_device_ioctl(br, BRCTL_ADD_IF, ifindex, 0, 0) < 0)
@@ -173,7 +145,7 @@ int br_set_path_cost(struct port *p, int path_cost)
 	return 0;
 }
 
-void __copy_fdb(struct fdb_entry *ent, struct __fdb_entry *f)
+static void __copy_fdb(struct fdb_entry *ent, const struct __fdb_entry *f)
 {
 	memcpy(ent->mac_addr, f->mac_addr, 6);
 	ent->port_no = f->port_no;
diff --git a/libbridge/libbridge_if.c b/libbridge/libbridge_if.c
index 6e08069..518d2b4 100644
--- a/libbridge/libbridge_if.c
+++ b/libbridge/libbridge_if.c
@@ -27,7 +27,7 @@
 #include "libbridge.h"
 #include "libbridge_private.h"
 
-static int br_ioctl32(unsigned long arg0, unsigned long arg1, unsigned long arg2)
+int br_get_br(unsigned long arg0, unsigned long arg1, unsigned long arg2)
 {
 	unsigned long arg[3];
 
@@ -38,54 +38,28 @@ static int br_ioctl32(unsigned long arg0, unsigned long arg1, unsigned long arg2
 	return ioctl(br_socket_fd, SIOCGIFBR, arg);
 }
 
-#ifdef __sparc__
-static int br_ioctl64(unsigned long arg0, unsigned long arg1, unsigned long arg2)
+int br_set_br(unsigned long arg0, unsigned long arg1, unsigned long arg2)
 {
-	unsigned long long arg[3];
+	unsigned long arg[3];
 
 	arg[0] = arg0;
 	arg[1] = arg1;
 	arg[2] = arg2;
 
-	return ioctl(br_socket_fd, SIOCGIFBR, arg);
-}
-
-int __kernel_is_64_bit()
-{
-	static int kernel_is_64_bit = -1;
-
-	if (kernel_is_64_bit == -1) {
-		struct utsname buf;
-
-		uname(&buf);
-		kernel_is_64_bit = !strcmp(buf.machine, "sparc64");
-	}
-
-	return kernel_is_64_bit;
-}
-#endif
-
-int br_ioctl(unsigned long arg0, unsigned long arg1, unsigned long arg2)
-{
-#ifdef __sparc__
-	if (__kernel_is_64_bit())
-		return br_ioctl64(arg0, arg1, arg2);
-#endif
-
-	return br_ioctl32(arg0, arg1, arg2);
+	return ioctl(br_socket_fd, SIOCSIFBR, arg);
 }
 
 int br_get_version(void)
 {
-	return br_ioctl(BRCTL_GET_VERSION, 0, 0);
+	return br_get_br(BRCTL_GET_VERSION, 0, 0);
 }
 
 int br_add_bridge(const char *brname)
 {
 	char _br[IFNAMSIZ];
 
-	memcpy(_br, brname, IFNAMSIZ);
-	if (br_ioctl(BRCTL_ADD_BRIDGE, (unsigned long)_br, 0) < 0)
+	strncpy(_br, brname, IFNAMSIZ);
+	if (br_set_br(BRCTL_ADD_BRIDGE, (unsigned long)_br, 0) < 0)
 		return errno;
 
 	return 0;
@@ -95,8 +69,8 @@ int br_del_bridge(const char *brname)
 {
 	char _br[IFNAMSIZ];
 
-	memcpy(_br, brname, IFNAMSIZ);
-	if (br_ioctl(BRCTL_DEL_BRIDGE, (unsigned long)_br, 0) < 0)
+	strncpy(_br, brname, IFNAMSIZ);
+	if (br_set_br(BRCTL_DEL_BRIDGE, (unsigned long)_br, 0) < 0)
 		return errno;
 
 	return 0;
diff --git a/libbridge/libbridge_init.c b/libbridge/libbridge_init.c
index e7bb918..e027457 100644
--- a/libbridge/libbridge_init.c
+++ b/libbridge/libbridge_init.c
@@ -26,10 +26,14 @@
 #include "libbridge.h"
 #include "libbridge_private.h"
 
+#define MAX_BRIDGES	1024	/* arbitrary */
+#define MAX_PORTS	256	/* STP limitation */
+
 int br_socket_fd;
 struct bridge *bridge_list;
 
-static void __bridge_info_copy(struct bridge_info *info, struct __bridge_info *i)
+static void __bridge_info_copy(struct bridge_info *info, 
+			       const struct __bridge_info *i)
 {
 	memcpy(&info->designated_root, &i->designated_root, 8);
 	memcpy(&info->bridge_id, &i->bridge_id, 8);
@@ -53,7 +57,8 @@ static void __bridge_info_copy(struct bridge_info *info, struct __bridge_info *i
 	__jiffies_to_tv(&info->gc_timer_value, i->gc_timer_value);
 }
 
-static void __port_info_copy(struct port_info *info, struct __port_info *i)
+static void __port_info_copy(struct port_info *info, 
+			     const struct __port_info *i)
 {
 	memcpy(&info->designated_root, &i->designated_root, 8);
 	memcpy(&info->designated_bridge, &i->designated_bridge, 8);
@@ -77,12 +82,16 @@ static int br_read_info(struct bridge *br)
 	struct __bridge_info i;
 
 	if (if_indextoname(br->ifindex, br->ifname) == NULL) {
-		return ENODEV;
+		/* index was there, but now it is gone! */
+		return -1;
 	}
 
 	if (br_device_ioctl(br, BRCTL_GET_BRIDGE_INFO,
-			    (unsigned long)&i, 0, 0) < 0)
+			    (unsigned long)&i, 0, 0) < 0) {
+		fprintf(stderr, "%s: can't get info %s\n",
+			br->ifname, strerror(errno));
 		return errno;
+	}
 
 	__bridge_info_copy(&br->info, &i);
 	return 0;
@@ -93,52 +102,58 @@ static int br_read_port_info(struct port *p)
 	struct __port_info i;
 
 	if (br_device_ioctl(p->parent, BRCTL_GET_PORT_INFO,
-			    (unsigned long)&i, p->index, 0) < 0)
+			    (unsigned long)&i, p->index, 0) < 0) {
+		fprintf(stderr, "%s: can't get port %d info %s\n",
+			p->parent->ifname, p->index, strerror(errno));
 		return errno;
+	}
 
 	__port_info_copy(&p->info, &i);
 	return 0;
 }
 
-void br_nuke_bridge(struct bridge *b)
+static void br_nuke_bridge(struct bridge *b)
 {
-	struct port *p;
+	struct port *p, *n;
 
-	p = b->firstport;
-	while (p != NULL) {
-		struct port *pnext;
-
-		pnext = p->next;
+	for (p = b->firstport; p; p = n) {
+		n = p->next;
 		free(p);
-		p = pnext;
 	}
 
 	free(b);
 }
 
-int br_make_port_list(struct bridge *br)
+static int br_make_port_list(struct bridge *br)
 {
 	int err;
 	int i;
-	int ifindices[256];
+	int ifindices[MAX_PORTS];
+	struct port *p, **top;
 
+	memset(ifindices, 0, sizeof(ifindices));
 	if (br_device_ioctl(br, BRCTL_GET_PORT_LIST, (unsigned long)ifindices,
-			    0, 0) < 0)
+			    MAX_PORTS, 0) < 0)
 		return errno;
 
-	for (i=255;i>=0;i--) {
-		struct port *p;
-
+	top = &br->firstport;
+	for (i = 0; i < MAX_PORTS; i++) {
 		if (!ifindices[i])
 			continue;
 
 		p = malloc(sizeof(struct port));
-		p->index = i;
+		if (!p) {
+			err = -ENOMEM;
+			goto error_out;
+		}
+
+		p->next = NULL;
 		p->ifindex = ifindices[i];
 		p->parent = br;
-		br->ports[i] = p;
-		p->next = br->firstport;
-		br->firstport = p;
+		p->index = i;
+		*top = p;
+		top = &p->next;
+
 		if ((err = br_read_port_info(p)) != 0)
 			goto error_out;
 	}
@@ -146,36 +161,46 @@ int br_make_port_list(struct bridge *br)
 	return 0;
 
  error_out:
-	while (++i < 256)
-		free(br->ports[i]);
+	p = br->firstport;
+	while (p) {
+		struct port *n = p->next;
+		free(p);
+		p = n;
+	}
+	br->firstport = NULL;
 
 	return err;
 }
 
-int br_make_bridge_list()
+static int br_make_bridge_list(void)
 {
-	int i;
-	int ifindices[32];
-	int num;
-
-	num = br_ioctl(BRCTL_GET_BRIDGES, (unsigned long)ifindices, 32);
-	if (num < 0)
+	int i, num;
+	int ifindices[MAX_BRIDGES];
+
+	num = br_get_br(BRCTL_GET_BRIDGES, (unsigned long)ifindices, 
+			MAX_BRIDGES);
+	if (num < 0) {
+		fprintf(stderr, "Get bridge indices failed: %s\n",
+			strerror(errno));
 		return errno;
-
+	}
 	bridge_list = NULL;
-	for (i=0;i<num;i++) {
+	for (i = 0;i < num; i++) {
 		struct bridge *br;
 
 		br = malloc(sizeof(struct bridge));
+		if (!br)
+			return -ENOMEM;
+
 		memset(br, 0, sizeof(struct bridge));
 		br->ifindex = ifindices[i];
 		br->firstport = NULL;
-		if ( br_read_info(br) != 0 ||
-		     br_make_port_list(br) != 0) {
+		if ( br_read_info(br) || br_make_port_list(br)) {
 			/* ignore the problem could just be a race! */
 			free(br);
 			continue;
 		}
+
 		br->next = bridge_list;
 		bridge_list = br;
 	}
@@ -190,8 +215,10 @@ int br_init()
 	if ((br_socket_fd = socket(AF_INET, SOCK_STREAM, 0)) < 0)
 		return errno;
 
-	if (br_get_version() != BRCTL_VERSION)
-		return 12345;
+	if (br_get_version() != BRCTL_VERSION) {
+		fprintf(stderr, "bridge utilities not compatiable with kernel version\n");
+		exit(1);
+	}
 
 	if ((err = br_make_bridge_list()) != 0)
 		return err;
@@ -211,7 +238,6 @@ int br_refresh()
 		br_nuke_bridge(b);
 		b = bnext;
 	}
-	bridge_list = NULL;
 
 	return br_make_bridge_list();
 }
diff --git a/libbridge/libbridge_misc.c b/libbridge/libbridge_misc.c
index 31e054e..c19d2fc 100644
--- a/libbridge/libbridge_misc.c
+++ b/libbridge/libbridge_misc.c
@@ -42,9 +42,10 @@ void __jiffies_to_tv(struct timeval *tv, unsigned long jiffies)
 	tv->tv_usec = tvusec - 1000000 * tv->tv_sec;
 }
 
-static char *state_names[5] = {"disabled", "listening", "learning", "forwarding", "blocking"};
+static const char *state_names[5] 
+= {"disabled", "listening", "learning", "forwarding", "blocking"};
 
-char *br_get_state_name(int state)
+const char *br_get_state_name(int state)
 {
 	if (state >= 0 && state <= 4)
 		return state_names[state];
@@ -52,7 +53,7 @@ char *br_get_state_name(int state)
 	return "<INVALID STATE>";
 }
 
-struct bridge *br_find_bridge(char *brname)
+struct bridge *br_find_bridge(const char *brname)
 {
 	struct bridge *b;
 
@@ -67,20 +68,19 @@ struct bridge *br_find_bridge(char *brname)
 	return NULL;
 }
 
-struct port *br_find_port(struct bridge *br, char *portname)
+
+struct port *br_find_port(struct bridge *br, const char *portname)
 {
-	char index;
+	int index;
 	struct port *p;
 
-	if (!(index = if_nametoindex(portname)))
+	index = if_nametoindex(portname);
+	if (index <= 0) 
 		return NULL;
 
-	p = br->firstport;
-	while (p != NULL) {
+	for (p = br->firstport; p; p = p->next) {
 		if (p->ifindex == index)
 			return p;
-
-		p = p->next;
 	}
 
 	return NULL;
diff --git a/libbridge/libbridge_private.h b/libbridge/libbridge_private.h
index 30f82d2..f2ea8b6 100644
--- a/libbridge/libbridge_private.h
+++ b/libbridge/libbridge_private.h
@@ -27,14 +27,8 @@ void __jiffies_to_tv(struct timeval *tv, unsigned long jiffies);
 int __kernel_is_64_bit(void);
 unsigned long __tv_to_jiffies(struct timeval *tv);
 
-int br_device_ioctl(struct bridge *br,
-		     unsigned long arg0,
-		     unsigned long arg1,
-		     unsigned long arg2,
-		     unsigned long arg3);
-int br_get_version(void);
-int br_ioctl(unsigned long arg0, unsigned long arg1, unsigned long arg2);
-int br_make_bridge_list(void);
-int br_make_port_list(struct bridge *br);
+int br_get_br(unsigned long arg0, unsigned long arg1, unsigned long arg2);
+int br_set_br(unsigned long arg0, unsigned long arg1, unsigned long arg2);
+int br_device_ioctl(struct bridge *br, unsigned long arg0, unsigned long arg1, unsigned long arg2, unsigned long arg3);
 
 #endif