Fix most compiler warnings in default GCC build.
Fix lots of warnings. Some const and type-pun breaks strict-aliasing
warnings left but much reduced.
* bgp_advertise.h: (struct bgp_advertise_fifo) is functionally identical to
(struct fifo), so just use that. Makes it clearer the beginning of
(struct bgp_advertise) is compatible with with (struct fifo), which seems
to be enough for gcc.
Add a BGP_ADV_FIFO_HEAD macro to contain the right cast to try shut up
type-punning breaks strict aliasing warnings.
* bgp_packet.c: Use BGP_ADV_FIFO_HEAD.
(bgp_route_refresh_receive) fix an interesting logic error in
(!ok || (ret != BLAH)) where ret is only well-defined if ok.
* bgp_vty.c: Peer commands should use bgp_vty_return to set their return.
* jhash.{c,h}: Can take const on * args without adding issues & fix warnings.
* libospf.h: LSA sequence numbers use the unsigned range of values, and
constants need to be set to unsigned, or it causes warnings in ospf6d.
* md5.h: signedness of caddr_t is implementation specific, change to an
explicit (uint_8 *), fix sign/unsigned comparison warnings.
* vty.c: (vty_log_fixed) const on level is well-intentioned, but not going
to fly given iov_base.
* workqueue.c: ALL_LIST_ELEMENTS_RO tests for null pointer, which is always
true for address of static variable. Correct but pointless warning in
this case, but use a 2nd pointer to shut it up.
* ospf6_route.h: Add a comment about the use of (struct prefix) to stuff 2
different 32 bit IDs into in (struct ospf6_route), and the resulting
type-pun strict-alias breakage warnings this causes. Need to use 2
different fields to fix that warning?
general:
* remove unused variables, other than a few cases where they serve a
sufficiently useful documentary purpose (e.g. for code that needs
fixing), or they're required dummies. In those cases, try mark them as
unused.
* Remove dead code that can't be reached.
* Quite a few 'no ...' forms of vty commands take arguments, but do not
check the argument matches the command being negated. E.g., should
'distance X <prefix>' succeed if previously 'distance Y <prefix>' was set?
Or should it be required that the distance match the previously configured
distance for the prefix?
Ultimately, probably better to be strict about this. However, changing
from slack to strict might expose problems in command aliases and tools.
* Fix uninitialised use of variables.
* Fix sign/unsigned comparison warnings by making signedness of types consistent.
* Mark functions as static where their use is restricted to the same compilation
unit.
* Add required headers
* Move constants defined in headers into code.
* remove dead, unused functions that have no debug purpose.
diff --git a/bgpd/bgp_advertise.h b/bgpd/bgp_advertise.h
index 4ebde90..2cf2a29 100644
--- a/bgpd/bgp_advertise.h
+++ b/bgpd/bgp_advertise.h
@@ -21,13 +21,6 @@
#ifndef _QUAGGA_BGP_ADVERTISE_H
#define _QUAGGA_BGP_ADVERTISE_H
-/* BGP advertise FIFO. */
-struct bgp_advertise_fifo
-{
- struct bgp_advertise *next;
- struct bgp_advertise *prev;
-};
-
/* BGP advertise attribute. */
struct bgp_advertise_attr
{
@@ -44,7 +37,7 @@
struct bgp_advertise
{
/* FIFO for advertisement. */
- struct bgp_advertise_fifo fifo;
+ struct fifo fifo;
/* Link list for same attribute advertise. */
struct bgp_advertise *next;
@@ -97,11 +90,13 @@
/* BGP advertisement list. */
struct bgp_synchronize
{
- struct bgp_advertise_fifo update;
- struct bgp_advertise_fifo withdraw;
- struct bgp_advertise_fifo withdraw_low;
+ struct fifo update;
+ struct fifo withdraw;
+ struct fifo withdraw_low;
};
+#define BGP_ADV_FIFO_HEAD(F) ((struct bgp_advertise *)FIFO_HEAD(F))
+
/* BGP adjacency linked list. */
#define BGP_INFO_ADD(N,A,TYPE) \
do { \
diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index fcf8255..b62a4f8 100644
--- a/bgpd/bgp_attr.c
+++ b/bgpd/bgp_attr.c
@@ -2110,7 +2110,6 @@
case SAFI_UNICAST:
case SAFI_MULTICAST:
{
- unsigned long sizep;
struct attr_extra *attre = attr->extra;
assert (attr->extra);
diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c
index b0cf2a9..3d2dade 100644
--- a/bgpd/bgp_mplsvpn.c
+++ b/bgpd/bgp_mplsvpn.c
@@ -84,7 +84,6 @@
struct prefix p;
int psize;
int prefixlen;
- u_int32_t label;
u_int16_t type;
struct rd_as rd_as;
struct rd_ip rd_ip;
@@ -117,8 +116,9 @@
zlog_err ("prefix length is less than 88: %d", prefixlen);
return -1;
}
-
- label = decode_label (pnt);
+
+ /* XXX: Not doing anything with the label */
+ decode_label (pnt);
/* Copyr label to prefix. */
tagpnt = pnt;;
diff --git a/bgpd/bgp_nexthop.c b/bgpd/bgp_nexthop.c
index 5b1d13a..6218e67 100644
--- a/bgpd/bgp_nexthop.c
+++ b/bgpd/bgp_nexthop.c
@@ -789,9 +789,9 @@
uint16_t length;
u_char marker;
u_char version;
- uint16_t command;
- int nbytes;
- struct in_addr raddr;
+ uint16_t command __attribute__((unused));
+ int nbytes __attribute__((unused));
+ struct in_addr raddr __attribute__((unused));
uint32_t metric;
int i;
u_char nexthop_num;
@@ -801,6 +801,7 @@
s = zlookup->ibuf;
stream_reset (s);
+ /* nbytes not being checked */
nbytes = stream_read (s, zlookup->sock, 2);
length = stream_getw (s);
@@ -814,9 +815,11 @@
__func__, zlookup->sock, marker, version);
return NULL;
}
-
+
+ /* XXX: not checking command */
command = stream_getw (s);
+ /* XXX: not doing anything with raddr */
raddr.s_addr = stream_get_ipv4 (s);
metric = stream_getl (s);
nexthop_num = stream_getc (s);
@@ -901,8 +904,6 @@
struct stream *s;
uint16_t length;
u_char version, marker;
- uint16_t command;
- int nbytes;
struct in6_addr raddr;
uint32_t metric;
int i;
@@ -913,10 +914,11 @@
s = zlookup->ibuf;
stream_reset (s);
- nbytes = stream_read (s, zlookup->sock, 2);
+ /* XXX: ignoring nbytes, see also zread_lookup */
+ stream_read (s, zlookup->sock, 2);
length = stream_getw (s);
- nbytes = stream_read (s, zlookup->sock, length - 2);
+ stream_read (s, zlookup->sock, length - 2);
marker = stream_getc (s);
version = stream_getc (s);
@@ -926,9 +928,11 @@
__func__, zlookup->sock, marker, version);
return NULL;
}
-
- command = stream_getw (s);
+ /* XXX: ignoring command */
+ stream_getw (s);
+
+ /* XXX: not actually doing anything with raddr */
stream_get (&raddr, s, 16);
metric = stream_getl (s);
@@ -1014,10 +1018,10 @@
{
struct stream *s;
int ret;
- u_int16_t length, command;
+ u_int16_t length, command __attribute__((unused));
u_char version, marker;
- int nbytes;
- struct in_addr addr;
+ int nbytes __attribute__((unused));
+ struct in_addr addr __attribute__((unused));
struct in_addr nexthop;
u_int32_t metric = 0;
u_char nexthop_num;
@@ -1063,6 +1067,7 @@
stream_reset (s);
/* Fetch length. */
+ /* XXX: not using nbytes */
nbytes = stream_read (s, zlookup->sock, 2);
length = stream_getw (s);
@@ -1077,9 +1082,11 @@
__func__, zlookup->sock, marker, version);
return 0;
}
-
+
+ /* XXX: not using command */
command = stream_getw (s);
+ /* XXX: not using addr */
addr.s_addr = stream_get_ipv4 (s);
metric = stream_getl (s);
nexthop_num = stream_getc (s);
diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c
index 65c6cac..0fab1b0 100644
--- a/bgpd/bgp_packet.c
+++ b/bgpd/bgp_packet.c
@@ -158,7 +158,7 @@
snlri = peer->scratch;
stream_reset (snlri);
- adv = FIFO_HEAD (&peer->sync[afi][safi]->update);
+ adv = BGP_ADV_FIFO_HEAD (&peer->sync[afi][safi]->update);
while (adv)
{
@@ -331,7 +331,6 @@
struct bgp_adj_out *adj;
struct bgp_advertise *adv;
struct bgp_node *rn;
- unsigned long pos;
bgp_size_t unfeasible_len;
bgp_size_t total_attr_len;
size_t mp_start = 0;
@@ -342,7 +341,7 @@
s = peer->work;
stream_reset (s);
- while ((adv = FIFO_HEAD (&peer->sync[afi][safi]->withdraw)) != NULL)
+ while ((adv = BGP_ADV_FIFO_HEAD (&peer->sync[afi][safi]->withdraw)) != NULL)
{
assert (adv->rn);
adj = adv->adj;
@@ -595,7 +594,7 @@
for (afi = AFI_IP; afi < AFI_MAX; afi++)
for (safi = SAFI_UNICAST; safi < SAFI_MAX; safi++)
{
- adv = FIFO_HEAD (&peer->sync[afi][safi]->withdraw);
+ adv = BGP_ADV_FIFO_HEAD (&peer->sync[afi][safi]->withdraw);
if (adv)
{
s = bgp_withdraw_packet (peer, afi, safi);
@@ -607,7 +606,7 @@
for (afi = AFI_IP; afi < AFI_MAX; afi++)
for (safi = SAFI_UNICAST; safi < SAFI_MAX; safi++)
{
- adv = FIFO_HEAD (&peer->sync[afi][safi]->update);
+ adv = BGP_ADV_FIFO_HEAD (&peer->sync[afi][safi]->update);
if (adv)
{
if (adv->binfo && adv->binfo->uptime < peer->synctime)
@@ -663,7 +662,7 @@
for (afi = AFI_IP; afi < AFI_MAX; afi++)
for (safi = SAFI_UNICAST; safi < SAFI_MAX; safi++)
- if ((adv = FIFO_HEAD (&peer->sync[afi][safi]->update)) != NULL)
+ if ((adv = BGP_ADV_FIFO_HEAD (&peer->sync[afi][safi]->update)) != NULL)
if (adv->binfo->uptime < peer->synctime)
return 1;
@@ -2036,7 +2035,6 @@
{
afi_t afi;
safi_t safi;
- u_char reserved;
struct stream *s;
/* If peer does not have the capability, send notification. */
@@ -2064,7 +2062,8 @@
/* Parse packet. */
afi = stream_getw (s);
- reserved = stream_getc (s);
+ /* reserved byte */
+ stream_getc (s);
safi = stream_getc (s);
if (BGP_DEBUG (normal, NORMAL))
@@ -2116,8 +2115,8 @@
if (orf_type == ORF_TYPE_PREFIX
|| orf_type == ORF_TYPE_PREFIX_OLD)
{
- u_char *p_pnt = stream_pnt (s);
- u_char *p_end = stream_pnt (s) + orf_len;
+ uint8_t *p_pnt = stream_pnt (s);
+ uint8_t *p_end = stream_pnt (s) + orf_len;
struct orf_prefix orfp;
u_char common = 0;
u_int32_t seq;
@@ -2157,7 +2156,7 @@
prefix_bgp_orf_remove_all (name);
break;
}
- ok = ((p_end - p_pnt) >= sizeof(u_int32_t)) ;
+ ok = ((size_t)(p_end - p_pnt) >= sizeof(u_int32_t)) ;
if (ok)
{
memcpy (&seq, p_pnt, sizeof (u_int32_t));
@@ -2209,8 +2208,8 @@
ret = prefix_bgp_orf_set (name, afi, &orfp,
(common & ORF_COMMON_PART_DENY ? 0 : 1 ),
(common & ORF_COMMON_PART_REMOVE ? 0 : 1));
-
- if (!ok || (ret != CMD_SUCCESS))
+
+ if (!ok || (ok && ret != CMD_SUCCESS))
{
if (BGP_DEBUG (normal, NORMAL))
zlog_debug ("%s Received misformatted prefixlist ORF."
@@ -2246,11 +2245,9 @@
struct capability_mp_data mpc;
struct capability_header *hdr;
u_char action;
- struct bgp *bgp;
afi_t afi;
safi_t safi;
- bgp = peer->bgp;
end = pnt + length;
while (pnt < end)
diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index 04cbb8a..e7357e5 100644
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -4525,20 +4525,11 @@
struct aspath *asmerge = NULL;
struct community *community = NULL;
struct community *commerge = NULL;
- struct in_addr nexthop;
- u_int32_t med = 0;
struct bgp_info *ri;
struct bgp_info *new;
int first = 1;
unsigned long match = 0;
- /* Record adding route's nexthop and med. */
- if (rinew)
- {
- nexthop = rinew->attr->nexthop;
- med = rinew->attr->med;
- }
-
/* ORIGIN attribute: If at least one route among routes that are
aggregated has ORIGIN with the value INCOMPLETE, then the
aggregated route must have the ORIGIN attribute with the value
@@ -4566,11 +4557,7 @@
continue;
if (! rinew && first)
- {
- nexthop = ri->attr->nexthop;
- med = ri->attr->med;
- first = 0;
- }
+ first = 0;
#ifdef AGGREGATE_NEXTHOP_CHECK
if (! IPV4_ADDR_SAME (&ri->attr->nexthop, &nexthop)
@@ -11773,7 +11760,13 @@
}
bdistance = rn->info;
-
+
+ if (bdistance->distance != distance)
+ {
+ vty_out (vty, "Distance does not match configured%s", VTY_NEWLINE);
+ return CMD_WARNING;
+ }
+
if (bdistance->access_list)
free (bdistance->access_list);
bgp_distance_free (bdistance);
diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c
index c498f58..06b0859 100644
--- a/bgpd/bgp_routemap.c
+++ b/bgpd/bgp_routemap.c
@@ -1174,7 +1174,6 @@
static void *
route_set_metric_compile (const char *arg)
{
- u_int32_t metric;
unsigned long larg;
char *endptr = NULL;
@@ -1185,7 +1184,6 @@
larg = strtoul (arg, &endptr, 10);
if (*endptr != '\0' || errno || larg > UINT32_MAX)
return NULL;
- metric = larg;
}
else
{
@@ -1199,7 +1197,6 @@
larg = strtoul (arg+1, &endptr, 10);
if (*endptr != '\0' || errno || larg > UINT32_MAX)
return NULL;
- metric = larg;
}
return XSTRDUP (MTYPE_ROUTE_MAP_COMPILED, arg);
@@ -1802,22 +1799,21 @@
route_match_ipv6_next_hop (void *rule, struct prefix *prefix,
route_map_object_t type, void *object)
{
- struct in6_addr *addr;
+ struct in6_addr *addr = rule;
struct bgp_info *bgp_info;
if (type == RMAP_BGP)
{
- addr = rule;
bgp_info = object;
if (!bgp_info->attr->extra)
return RMAP_NOMATCH;
- if (IPV6_ADDR_SAME (&bgp_info->attr->extra->mp_nexthop_global, rule))
+ if (IPV6_ADDR_SAME (&bgp_info->attr->extra->mp_nexthop_global, addr))
return RMAP_MATCH;
if (bgp_info->attr->extra->mp_nexthop_len == 32 &&
- IPV6_ADDR_SAME (&bgp_info->attr->extra->mp_nexthop_local, rule))
+ IPV6_ADDR_SAME (&bgp_info->attr->extra->mp_nexthop_local, addr))
return RMAP_MATCH;
return RMAP_NOMATCH;
@@ -3430,7 +3426,7 @@
"IP address of aggregator\n")
{
int ret;
- as_t as;
+ as_t as __attribute__((unused)); /* dummy for VTY_GET_INTEGER_RANGE */
struct in_addr address;
char *argstr;
@@ -3464,7 +3460,7 @@
"AS number of aggregator\n")
{
int ret;
- as_t as;
+ as_t as __attribute__((unused)); /* dummy for VTY_GET_INTEGER_RANGE */
struct in_addr address;
char *argstr;
diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c
index a818fe7..ca44774 100644
--- a/bgpd/bgp_vty.c
+++ b/bgpd/bgp_vty.c
@@ -576,7 +576,7 @@
"AS number\n")
{
struct bgp *bgp;
- as_t as;
+ as_t as __attribute__((unused)); /* Dummy for VTY_GET_INTEGER_RANGE */
bgp = vty->index;
@@ -3205,7 +3205,6 @@
peer_weight_set_vty (struct vty *vty, const char *ip_str,
const char *weight_str)
{
- int ret;
struct peer *peer;
unsigned long weight;
@@ -3215,9 +3214,7 @@
VTY_GET_INTEGER_RANGE("weight", weight, weight_str, 0, 65535);
- ret = peer_weight_set (peer, weight);
-
- return CMD_SUCCESS;
+ return bgp_vty_return (vty, peer_weight_set (peer, weight));
}
static int
@@ -3229,9 +3226,7 @@
if (! peer)
return CMD_WARNING;
- peer_weight_unset (peer);
-
- return CMD_SUCCESS;
+ return bgp_vty_return (vty, peer_weight_unset (peer));
}
DEFUN (neighbor_weight,
@@ -3371,7 +3366,6 @@
peer_timers_connect_set_vty (struct vty *vty, const char *ip_str,
const char *time_str)
{
- int ret;
struct peer *peer;
u_int32_t connect;
@@ -3381,24 +3375,19 @@
VTY_GET_INTEGER_RANGE ("Connect time", connect, time_str, 0, 65535);
- ret = peer_timers_connect_set (peer, connect);
-
- return CMD_SUCCESS;
+ return bgp_vty_return (vty, peer_timers_connect_set (peer, connect));
}
static int
peer_timers_connect_unset_vty (struct vty *vty, const char *ip_str)
{
- int ret;
struct peer *peer;
peer = peer_and_group_lookup_vty (vty, ip_str);
if (! peer)
return CMD_WARNING;
- ret = peer_timers_connect_unset (peer);
-
- return CMD_SUCCESS;
+ return bgp_vty_return (vty, peer_timers_connect_unset (peer));
}
DEFUN (neighbor_timers_connect,
@@ -3455,7 +3444,7 @@
else
ret = peer_advertise_interval_unset (peer);
- return CMD_SUCCESS;
+ return bgp_vty_return (vty, ret);
}
DEFUN (neighbor_advertise_interval,
@@ -3505,7 +3494,7 @@
else
ret = peer_interface_unset (peer);
- return CMD_SUCCESS;
+ return bgp_vty_return (vty, ret);
}
DEFUN (neighbor_interface,