From 750e814693050bc97391eec618aad9db798ee5e8 Mon Sep 17 00:00:00 2001 From: Paul Jakma Date: Tue, 22 Jul 2008 21:11:48 +0000 Subject: [bgpd] Fix triggerable crash when compiled with --disable-bgp-announce 2008-07-22 Paul Jakma * HACKING: Document preference for compiler conditional code, over cpp conditional. * configure.ac: DISABLE_BGP_ANNOUNCE always should be defined. * bgp_{packet,route,advertise}.c: change to compiler testing of DISABLE_BGP_ANNOUNCE, rather than cpp. 2008-07-22 MIYAJIMA Mitsuharu * bgp_packet.c: (bgp_update_packet_eor) Fix crash triggerable if a bgpd was compiled with --disable-bgp-announce and if GR is advertised by peer. --- ChangeLog | 6 ++++++ HACKING | 24 +++++++++++++++++++++++- bgpd/ChangeLog | 11 +++++++++++ bgpd/bgp_advertise.c | 10 ++++------ bgpd/bgp_packet.c | 20 ++++++++------------ bgpd/bgp_route.c | 10 ++++------ configure.ac | 4 +++- 7 files changed, 59 insertions(+), 26 deletions(-) diff --git a/ChangeLog b/ChangeLog index 245dd133..cf5c774d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2008-07-22 Paul Jakma + + * HACKING: Document preference for compiler conditional code, over + cpp conditional. + * configure.ac: DISABLE_BGP_ANNOUNCE always should be defined. + 2008-06-10 Paul Jakma * configure.ac: Bump version to 0.99.10 diff --git a/HACKING b/HACKING index 7c92604a..ec9aa7c0 100644 --- a/HACKING +++ b/HACKING @@ -1,5 +1,5 @@ -*- mode: text; -*- -$Id: HACKING,v 1.21 2005/11/10 10:21:19 paul Exp $ +$Id$ GUIDELINES FOR HACKING ON QUAGGA @@ -75,6 +75,28 @@ release. See also below regarding SHARED LIBRARY VERSIONING. +COMPILE-TIME CONDITIONAL CODE + +Please think very carefully before making code conditional at compile time, +as it increases maintenance burdens and user confusion. In particular, +please avoid gratuitious --enable-.... switches to the configure script - +typically code should be good enough to be in Quagga, or it shouldn't be +there at all. + +When code must be compile-time conditional, try have the compiler make it +conditional rather than the C pre-processor. I.e. this: + + if (SOME_SYMBOL) + frobnicate(); + +rather than: + + #ifdef SOME_SYMBOL + frobnicate (); + #endif /* SOME_SYMBOL */ + +Note that the former approach requires ensuring that SOME_SYMBOL will be +defined (watch your AC_DEFINEs). CHANGELOG diff --git a/bgpd/ChangeLog b/bgpd/ChangeLog index 24244947..6fe2b0f5 100644 --- a/bgpd/ChangeLog +++ b/bgpd/ChangeLog @@ -1,3 +1,14 @@ +2008-07-22 Paul Jakma + + * bgp_{packet,route,advertise}.c: change to compiler testing of + DISABLE_BGP_ANNOUNCE, rather than cpp. + +2008-07-22 MIYAJIMA Mitsuharu + + * bgp_packet.c: (bgp_update_packet_eor) Fix crash triggerable + if a bgpd was compiled with --disable-bgp-announce and if GR is + advertised by peer. + 2008-07-22 Paul Jakma * bgp_community.c: (community_str2com) assigns defaults to local diff --git a/bgpd/bgp_advertise.c b/bgpd/bgp_advertise.c index 73b868a8..870aab13 100644 --- a/bgpd/bgp_advertise.c +++ b/bgpd/bgp_advertise.c @@ -220,9 +220,8 @@ bgp_adj_out_set (struct bgp_node *rn, struct peer *peer, struct prefix *p, struct bgp_adj_out *adj = NULL; struct bgp_advertise *adv; -#ifdef DISABLE_BGP_ANNOUNCE - return; -#endif /* DISABLE_BGP_ANNOUNCE */ + if (DISABLE_BGP_ANNOUNCE) + return; /* Look for adjacency information. */ if (rn) @@ -274,9 +273,8 @@ bgp_adj_out_unset (struct bgp_node *rn, struct peer *peer, struct prefix *p, struct bgp_adj_out *adj; struct bgp_advertise *adv; -#ifdef DISABLE_BGP_ANNOUNCE - return; -#endif /* DISABLE_BGP_ANNOUNCE */ + if (DISABLE_BGP_ANNOUNCE) + return; /* Lookup existing adjacency, if it is not there return immediately. */ for (adj = rn->adj_out; adj; adj = adj->next) diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c index 623fd9ec..4d7f32de 100644 --- a/bgpd/bgp_packet.c +++ b/bgpd/bgp_packet.c @@ -235,9 +235,8 @@ bgp_update_packet_eor (struct peer *peer, afi_t afi, safi_t safi) struct stream *s; struct stream *packet; -#ifdef DISABLE_BGP_ANNOUNCE - return; -#endif /* DISABLE_BGP_ANNOUNCE */ + if (DISABLE_BGP_ANNOUNCE) + return NULL; if (BGP_DEBUG (normal, NORMAL)) zlog_debug ("send End-of-RIB for %s to %s", afi_safi_print (afi, safi), peer->host); @@ -369,9 +368,8 @@ bgp_default_update_send (struct peer *peer, struct attr *attr, char attrstr[BUFSIZ]; char buf[BUFSIZ]; -#ifdef DISABLE_BGP_ANNOUNCE - return; -#endif /* DISABLE_BGP_ANNOUNCE */ + if (DISABLE_BGP_ANNOUNCE) + return; if (afi == AFI_IP) str2prefix ("0.0.0.0/0", &p); @@ -438,9 +436,8 @@ bgp_default_withdraw_send (struct peer *peer, afi_t afi, safi_t safi) bgp_size_t total_attr_len; char buf[BUFSIZ]; -#ifdef DISABLE_BGP_ANNOUNCE - return; -#endif /* DISABLE_BGP_ANNOUNCE */ + if (DISABLE_BGP_ANNOUNCE) + return; if (afi == AFI_IP) str2prefix ("0.0.0.0/0", &p); @@ -958,9 +955,8 @@ bgp_route_refresh_send (struct peer *peer, afi_t afi, safi_t safi, struct bgp_filter *filter; int orf_refresh = 0; -#ifdef DISABLE_BGP_ANNOUNCE - return; -#endif /* DISABLE_BGP_ANNOUNCE */ + if (DISABLE_BGP_ANNOUNCE) + return; filter = &peer->filter[afi][safi]; diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 4fbc4bab..b639db05 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -742,9 +742,8 @@ bgp_announce_check (struct bgp_info *ri, struct peer *peer, struct prefix *p, filter = &peer->filter[afi][safi]; bgp = peer->bgp; -#ifdef DISABLE_BGP_ANNOUNCE - return 0; -#endif + if (DISABLE_BGP_ANNOUNCE) + return 0; /* Do not send announces to RS-clients from the 'normal' bgp_table. */ if (CHECK_FLAG(peer->af_flags[afi][safi], PEER_FLAG_RSERVER_CLIENT)) @@ -1095,9 +1094,8 @@ bgp_announce_check_rsclient (struct bgp_info *ri, struct peer *rsclient, filter = &rsclient->filter[afi][safi]; bgp = rsclient->bgp; -#ifdef DISABLE_BGP_ANNOUNCE - return 0; -#endif + if (DISABLE_BGP_ANNOUNCE) + return 0; /* Do not send back route to sender. */ if (from == rsclient) diff --git a/configure.ac b/configure.ac index 9cebf48d..746b5cea 100755 --- a/configure.ac +++ b/configure.ac @@ -1207,7 +1207,9 @@ case "${enable_solaris}" in esac if test "${enable_bgp_announce}" = "no";then - AC_DEFINE(DISABLE_BGP_ANNOUNCE,,Disable BGP installation to zebra) + AC_DEFINE(DISABLE_BGP_ANNOUNCE,1,Disable BGP installation to zebra) +else + AC_DEFINE(DISABLE_BGP_ANNOUNCE,0,Disable BGP installation to zebra) fi AC_SUBST(ZEBRA) -- cgit v1.2.1