[bgpd] Fix triggerable crash when compiled with --disable-bgp-announce

2008-07-22 Paul Jakma <paul.jakma@sun.com>

	* 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 <miyajima.mitsuharu@anchor.jp>

	* 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.

diff --git a/ChangeLog b/ChangeLog
index 245dd133155051b7beec6a527e7afe957caeb455..cf5c774d757fbb282d526df0bf46a3a74e15d60a 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2008-07-22 Paul Jakma <paul.jakma@sun.com>
+
+       * HACKING: Document preference for compiler conditional code, over
+         cpp conditional.
+       * configure.ac: DISABLE_BGP_ANNOUNCE always should be defined.
+
 2008-06-10 Paul Jakma <paul@jakma.org>
 
        * configure.ac: Bump version to 0.99.10

diff --git a/HACKING b/HACKING
index 7c92604ac9e0ba8dab33b23e940c5fae1a456f37..ec9aa7c0912b326d2d09df5916f99d26e3ec3e82 100644 (file)
--- 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 242449473ae689a5b100c7b68f958a14eb115659..6fe2b0f5ae8984f5e248e2f03a8d59d80dc491f2 100644 (file)
--- a/bgpd/ChangeLog
+++ b/bgpd/ChangeLog
@@ -1,3 +1,14 @@
+2008-07-22 Paul Jakma <paul.jakma@sun.com>
+
+       * bgp_{packet,route,advertise}.c: change to compiler testing of
+         DISABLE_BGP_ANNOUNCE, rather than cpp.
+
+2008-07-22 MIYAJIMA Mitsuharu <miyajima.mitsuharu@anchor.jp>
+
+       * 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 <paul.jakma@sun.com>
 
        * bgp_community.c: (community_str2com) assigns defaults to local

diff --git a/bgpd/bgp_advertise.c b/bgpd/bgp_advertise.c
index 73b868a82b0218f72e6705ada8564c7006ef17ef..870aab134d13248bbffdcb7b447e59549386c5e4 100644 (file)
--- 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 623fd9ec0ae0b35476d5305c3e028c90843abaf0..4d7f32dee96945fae8be54cfebcec3e7c9d24e7b 100644 (file)
--- 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 4fbc4babde9b8f4c41301ecc6607c72ca2d865b8..b639db05c2c3f08b4753e7e79ae872f35021ef3c 100644 (file)
--- 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 9cebf48d9e056dbabfb536f948fdfea4f6eb9d4f..746b5cea5ac399b956c3d0471f83795ca971ec77 100755 (executable)
--- 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)