From 08b6a617e862ffc7b7460abb79fe4c198806517c Mon Sep 17 00:00:00 2001
From: "Ondrej Zajicek (work)" <santiago@crfreenet.org>
Date: Tue, 29 Aug 2017 19:17:35 +0200
Subject: [PATCH] RAdv: Some style nitpicks

---
 doc/bird.sgml        |  8 +++---
 proto/radv/packets.c | 10 +++----
 proto/radv/radv.c    | 64 ++++++++++++++++++++++++--------------------
 proto/radv/radv.h    | 16 +++++------
 4 files changed, 50 insertions(+), 48 deletions(-)

diff --git a/doc/bird.sgml b/doc/bird.sgml
index d8dad431..516f4c6e 100644
--- a/doc/bird.sgml
+++ b/doc/bird.sgml
@@ -3437,10 +3437,10 @@ dsc-iface
 	Default: 3 * <cf/max ra	interval/, <cf/sensitive/ yes.
 
 	<tag><label id="radv-iface-linger-time">linger time <m/expr/</tag>
-	When a prefix disappears, it is advertised for some time with 0 lifetime,
-	to inform clients the prefix is no longer usable. This sets the time for
-	how long it is advertised (in seconds). Maximum is 3600, 0 means
-	disable. Default 300.
+	When a prefix disappears, it is advertised for some time with 0
+	lifetime, to inform clients the prefix is no longer usable. This option
+	sets the time for how long it is advertised (in seconds). Maximum is
+	3600, 0 means disabled. Default: 300.
 
 	<tag><label id="radv-iface-default-preference-low">default preference low|medium|high</tag>
 	This option specifies the Default Router Preference value to advertise
diff --git a/proto/radv/packets.c b/proto/radv/packets.c
index 088d177c..19d71f97 100644
--- a/proto/radv/packets.c
+++ b/proto/radv/packets.c
@@ -209,16 +209,16 @@ static int
 radv_prepare_prefix(struct radv_iface *ifa, struct radv_prefix *prefix,
 		    char **buf, char *bufend)
 {
-  struct radv_prefix_config *pc = prefix->config;
-  struct radv_opt_prefix *op = (void *) *buf;
+  struct radv_prefix_config *pc = prefix->cf;
 
-  if (*buf + sizeof(*op) > bufend)
+  if (*buf + sizeof(struct radv_opt_prefix) > bufend)
   {
-    log(L_WARN "%s: Too many prefixes on interface %s", ifa->ra->p.name,
-      ifa->iface->name);
+    log(L_WARN "%s: Too many prefixes on interface %s",
+	ifa->ra->p.name, ifa->iface->name);
     return -1;
   }
 
+  struct radv_opt_prefix *op = (void *) *buf;
   op->type = OPT_PREFIX;
   op->length = 4;
   op->pxlen = prefix->len;
diff --git a/proto/radv/radv.c b/proto/radv/radv.c
index 44040a4b..227c8ef6 100644
--- a/proto/radv/radv.c
+++ b/proto/radv/radv.c
@@ -51,13 +51,14 @@ radv_timer(timer *tm)
 
   RADV_TRACE(D_EVENTS, "Timer fired on %s", ifa->iface->name);
 
-  /* If some dead prefixes expired, regenerate the prefix list and the packet.
+  /*
+   * If some dead prefixes expired, regenerate the prefix list and the packet.
    * We do so by pretending there was a change on the interface.
    *
    * This sets the timer, but we replace it just at the end of this function
    * (replacing a timer is fine).
    */
-  if (ifa->prefix_expires != 0 && ifa->prefix_expires <= now)
+  if (ifa->prefix_expires && (ifa->prefix_expires <= now))
     radv_iface_notify(ifa, RA_EV_GC);
 
   radv_send_ra(ifa, 0);
@@ -110,20 +111,20 @@ radv_prefix_match(struct radv_iface *ifa, struct ifa *a)
 
 /*
  * Go through the list of prefixes, compare them with configs and decide if we
- * want them or not. */
+ * want them or not.
+ */
 static void
-prefixes_prepare(struct radv_iface *ifa)
+radv_prepare_prefixes(struct radv_iface *ifa)
 {
   struct radv_proto *p = ifa->ra;
   struct radv_iface_config *cf = ifa->cf;
-  /* First mark all the prefixes as unused */
   struct radv_prefix *pfx;
 
+  /* First mark all the prefixes as unused */
   WALK_LIST(pfx, ifa->prefixes)
     pfx->mark = 0;
 
-  /* Now find all the prefixes we want to use and make sure they are in the
-   * list. */
+  /* Find all the prefixes we want to use and make sure they are in the list. */
   struct ifa *addr;
   WALK_LIST(addr, ifa->iface->addrs)
   {
@@ -135,8 +136,7 @@ prefixes_prepare(struct radv_iface *ifa)
     /* Do we have it already? */
     struct radv_prefix *existing = NULL;
     WALK_LIST(pfx, ifa->prefixes)
-      if (pfx->len == addr->pxlen &&
-	  memcmp(&pfx->prefix, &addr->prefix, sizeof pfx->prefix) == 0)
+      if ((pfx->len == addr->pxlen) && ipa_equal(pfx->prefix, addr->prefix))
       {
 	existing = pfx;
 	break;
@@ -144,20 +144,22 @@ prefixes_prepare(struct radv_iface *ifa)
 
     if (!existing)
     {
-      RADV_TRACE(D_EVENTS, "Allocating new prefix %I on %s", addr->prefix,
-		 ifa->iface->name);
+      RADV_TRACE(D_EVENTS, "Adding new prefix %I/%d on %s",
+		 addr->prefix, addr->pxlen, ifa->iface->name);
+
       existing = mb_allocz(ifa->pool, sizeof *existing);
       existing->prefix = addr->prefix;
       existing->len = addr->pxlen;
       add_tail(&ifa->prefixes, NODE existing);
     }
+
     /*
      * Update the information (it may have changed, or even bring a prefix back
      * to life).
      */
     existing->alive = 1;
     existing->mark = 1;
-    existing->config = pc;
+    existing->cf = pc;
   }
 
   /*
@@ -165,37 +167,41 @@ prefixes_prepare(struct radv_iface *ifa)
    * dropped just yet). If something is dead and rots there for long enough,
    * clean it up.
    */
-  bird_clock_t rotten = now + cf->linger_time;
+  bird_clock_t expires = now + cf->linger_time;
+  bird_clock_t expires_min = 0;
   struct radv_prefix *next;
-  bird_clock_t expires_soonest = 0;
-  WALK_LIST_DELSAFE(pfx, next, ifa->prefixes) {
+  WALK_LIST_DELSAFE(pfx, next, ifa->prefixes)
+  {
     if (pfx->alive && !pfx->mark)
     {
-      RADV_TRACE(D_EVENTS, "Marking prefix %I on %s as dead", pfx->prefix,
-		 ifa->iface->name);
-      // It just died
+      RADV_TRACE(D_EVENTS, "Marking prefix %I/$d on %s as dead",
+		 pfx->prefix, pfx->len, ifa->iface->name);
+
       pfx->alive = 0;
-      pfx->expires = rotten;
-      pfx->config = &dead_prefix;
+      pfx->expires = expires;
+      pfx->cf = &dead_prefix;
     }
+
     if (!pfx->alive)
+    {
       if (pfx->expires <= now)
       {
-	RADV_TRACE(D_EVENTS, "Dropping long dead prefix %I on %s", pfx->prefix,
-		   ifa->iface->name);
-	// It's dead and rotten, clean it up
+	RADV_TRACE(D_EVENTS, "Removing prefix %I/%d on %s",
+		   pfx->prefix, pfx->len, ifa->iface->name);
+
 	rem_node(NODE pfx);
 	mb_free(pfx);
       }
       else
       {
-	ASSERT(pfx->expires != 0);
-	// Let it rot for a while more, but look when it's ripe.
-	if (expires_soonest == 0 || pfx->expires < expires_soonest)
-	  expires_soonest = pfx->expires;
+	/* Find minimum expiration time */
+	if (!expires_min || (pfx->expires < expires_min))
+	  expires_min = pfx->expires;
       }
+    }
   }
-  ifa->prefix_expires = expires_soonest;
+
+  ifa->prefix_expires = expires_min;
 }
 
 static char* ev_name[] = { NULL, "Init", "Change", "RS", "Garbage collect" };
@@ -223,7 +229,7 @@ radv_iface_notify(struct radv_iface *ifa, int event)
     break;
   }
 
-  prefixes_prepare(ifa);
+  radv_prepare_prefixes(ifa);
 
   /* Update timer */
   unsigned delta = now - ifa->last;
diff --git a/proto/radv/radv.h b/proto/radv/radv.h
index 777c6e16..60b9980f 100644
--- a/proto/radv/radv.h
+++ b/proto/radv/radv.h
@@ -130,16 +130,12 @@ struct radv_prefix		/* One prefix we advertise */
   node n;
   ip_addr prefix;
   u8 len;
-  /* Is the prefix alive? If not, we advertise it with 0 lifetime, so clients
-   * stop using it. */
-  u8 alive;
+  u8 alive;			/* Is the prefix alive? If not, we advertise it
+				   with 0 lifetime, so clients stop using it */
   u8 mark;			/* A temporary mark for processing */
-  /* The (absolute) time when we drop this prefix from advertising. It is valid
-   * only if !alive. */
-  bird_clock_t expires;
-  /* The config tied to this prefix. Always valid (we place a dummy config here
-   * when !alive). */
-  struct radv_prefix_config *config;
+  bird_clock_t expires;		/* The time when we drop this prefix from
+				   advertising. It is valid only if !alive. */
+  struct radv_prefix_config *cf; /* The config tied to this prefix */
 };
 
 struct radv_iface
@@ -150,7 +146,7 @@ struct radv_iface
   struct iface *iface;
   struct ifa *addr;		/* Link-local address of iface */
   struct pool *pool;		/* A pool for interface-specific things */
-  list prefixes;		/* The prefixes we advertise */
+  list prefixes;		/* The prefixes we advertise (struct radv_prefix) */
   bird_clock_t prefix_expires;	/* When the soonest prefix expires (0 = none dead) */
 
   timer *timer;