[quagga-dev 12180] [PATCH] ospfd: Make ospf_passive_interface_update calls friendly to static analysis

Paul Jakma paul at opensourcerouting.org
Mon Apr 27 16:25:34 BST 2015


This one covers up a false positive in either clang or Coverity's SA - I
don't quite remember which. Trivial, but probably also easier for a human to
follow.

* ospf_vty.c: ({no_}ospf_passive_interface_addr_cmd) To a static analyser,
  the call to ospf_passive_interface_update can look like uninitialised memory
  in addr might be read from. It won't be, as ospf_passive_interface_update
  only reads addr if params != IF_DEF_PARAMS, but not clear. Split up the
  helper into the two cases to make it clear.
---
 ospfd/ospf_vty.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/ospfd/ospf_vty.c b/ospfd/ospf_vty.c
index 72493a2..2fcdedc 100644
--- a/ospfd/ospf_vty.c
+++ b/ospfd/ospf_vty.c
@@ -254,9 +254,9 @@ ospf_passive_interface_default (struct ospf *ospf, u_char newval)
 }
 
 static void
-ospf_passive_interface_update (struct ospf *ospf, struct interface *ifp,
-                               struct in_addr addr, 
-                               struct ospf_if_params *params, u_char value)
+ospf_passive_interface_update_addr (struct ospf *ospf, struct interface *ifp,
+                               struct ospf_if_params *params, u_char value,
+                               struct in_addr addr)
 {
   u_char dflt;
   
@@ -276,7 +276,14 @@ ospf_passive_interface_update (struct ospf *ospf, struct interface *ifp,
       ospf_free_if_params (ifp, addr);
       ospf_if_update_params (ifp, addr);
     }
-  else
+}
+
+static void
+ospf_passive_interface_update (struct ospf *ospf, struct interface *ifp,
+                               struct ospf_if_params *params, u_char value)
+{
+  params->passive_interface = value;
+  if (params == IF_DEF_PARAMS (ifp))
     {
       if (value != ospf->passive_interface_default)
         SET_IF_PARAM (params, passive_interface);
@@ -320,8 +327,11 @@ DEFUN (ospf_passive_interface,
 
       params = ospf_get_if_params (ifp, addr);
       ospf_if_update_params (ifp, addr);
+      ospf_passive_interface_update_addr (ospf, ifp, params,
+                                          OSPF_IF_PASSIVE, addr);
     }
-  ospf_passive_interface_update (ospf, ifp, addr, params, OSPF_IF_PASSIVE);
+  
+  ospf_passive_interface_update (ospf, ifp, params, OSPF_IF_PASSIVE);
 
   /* XXX We should call ospf_if_set_multicast on exactly those
    * interfaces for which the passive property changed.  It is too much
@@ -397,9 +407,11 @@ DEFUN (no_ospf_passive_interface,
       params = ospf_lookup_if_params (ifp, addr);
       if (params == NULL)
 	return CMD_SUCCESS;
+      ospf_passive_interface_update_addr (ospf, ifp, params, OSPF_IF_ACTIVE, 
+                                          addr);
     }
-  ospf_passive_interface_update (ospf, ifp, addr, params, OSPF_IF_ACTIVE);
-
+  ospf_passive_interface_update (ospf, ifp, params, OSPF_IF_ACTIVE);
+  
   /* XXX We should call ospf_if_set_multicast on exactly those
    * interfaces for which the passive property changed.  It is too much
    * work to determine this set, so we do this for every interface.
-- 
2.1.0





More information about the Quagga-dev mailing list