*: fix remaining vty->index & add coccinelle patch

Fix instances that the coccinelle patch didn't catch (or which coccinelle couldn't parse), and add the patch file for future posterity (and application to yet-unmerged branches). Signed-off-by: David Lamparter <[email protected]>
jbonor · Dec 9, 2016 · 52c6b0e · 52c6b0e
1 parent cdc2d76
commit 52c6b0e
Show file tree

Hide file tree

Showing 6 changed files with 303 additions and 7 deletions.
diff --git a/bgpd/rfp-example/librfp/rfp_example.c b/bgpd/rfp-example/librfp/rfp_example.c
@@ -45,7 +45,7 @@ DEFUN (rfp_example_config_value,
 {
   uint32_t value = 0;
   struct rfp_instance_t *rfi = NULL;
-  rfi = rfapi_get_rfp_start_val (vty->index);   /* index=bgp for BGP_NODE */
+  rfi = rfapi_get_rfp_start_val (VTY_GET_CONTEXT (bgp));   /* BGP_NODE */
   assert (rfi != NULL);
 
   VTY_GET_INTEGER ("Example value", value, argv[2]->arg);

diff --git a/ospfd/ospf_te.c b/ospfd/ospf_te.c
@@ -2304,12 +2304,9 @@ DEFUN (no_ospf_mpls_te,
        MPLS_TE_STR
        "Disable the MPLS-TE functionality\n")
 {
+  VTY_DECLVAR_CONTEXT(ospf, ospf);
   struct listnode *node, *nnode;
   struct mpls_te_link *lp;
-  struct ospf *ospf = vty->index;
-
-  if (!ospf)
-    return CMD_SUCCESS;
 
   if (OspfMplsTE.status == disabled)
     return CMD_SUCCESS;

diff --git a/ospfd/ospf_vty.c b/ospfd/ospf_vty.c
@@ -163,7 +163,7 @@ DEFUN (router_ospf,
   if (argc > 2)
     VTY_GET_INTEGER ("Instance", instance, argv[2]->arg);
 
-  /* The following logic to set the vty->index is in place to be able
+  /* The following logic to set the vty qobj index is in place to be able
      to ignore the commands which dont belong to this instance. */
   if (ospf->instance != instance)
     VTY_PUSH_CONTEXT_NULL(OSPF_NODE);
@@ -172,7 +172,7 @@ DEFUN (router_ospf,
       if (IS_DEBUG_OSPF_EVENT)
         zlog_debug ("Config command 'router ospf %d' received", instance);
       ospf->oi_running = 1;
-      vty->index = ospf;
+      VTY_PUSH_CONTEXT(OSPF_NODE, ospf);
       ospf_router_id_update (ospf);
     }
 

diff --git a/tools/cocci.h b/tools/cocci.h
@@ -0,0 +1,34 @@
+/* some of this stuff doesn't seem to parse properly in coccinelle */
+
+#define DEFUN(funcname, cmdname, str, help) \
+  static int funcname \
+    (const struct cmd_element *self, \
+     struct vty *vty, \
+     int argc, \
+     struct cmd_token *argv[])
+#define DEFUN_HIDDEN(funcname, cmdname, str, help) \
+  static int funcname \
+    (const struct cmd_element *self, \
+     struct vty *vty, \
+     int argc, \
+     struct cmd_token *argv[])
+
+#define ENABLE_BGP_VNC 1
+#define ALL_LIST_ELEMENTS_RO(list,node,data) \
+  (node) = listhead(list), ((data) = NULL);\
+  (node) != NULL && ((data) = listgetdata(node)); \
+  (node) = listnextnode(node), ((data) = NULL)
+#define ALL_LIST_ELEMENTS(list,node,nextnode,data) \
+  (node) = listhead(list), ((data) = NULL); \
+  (node) != NULL && \
+    ((data) = listgetdata(node),(nextnode) = node->next); \
+  (node) = (nextnode), ((data) = NULL)
+#define LIST_HEAD(name, type) \
+        struct name { \
+                struct type *lh_first;  /* first element */ \
+        }
+#define     LIST_ENTRY(type)                                        \
+struct {                                                            \
+    struct type *le_next;   /* next element */                      \
+    struct type **le_prev;  /* address of previous next element */  \
+}
diff --git a/tools/vty_index.cocci b/tools/vty_index.cocci
@@ -0,0 +1,244 @@
+/*
+ * prep: strip off casts, they cause things to fail matching later.
+ */
+
+@@
+identifier casttarget;
+symbol vty;
+@@
+
+- (struct casttarget *)vty->index
++ vty->index
+
+/*
+ * variant 1:  local variable assigned from vty->index
+ */
+
+@@
+identifier sn, nn;
+identifier fn;
+@@
+
+  int fn(...)
+  {
++ VTY_DECLVAR_CONTEXT (sn, nn);
+  ...
+  \(
+-   struct sn *nn;
+    ...
+-   nn = vty->index;
+  \|
+-   struct sn *nn = vty->index;
+  \|
+-   struct sn *nn = vty->index;
+    ...
+-   nn = vty->index;
+  \)
+  ...
+  }
+
+@@
+identifier sn, nn;
+identifier fn;
+type Tr;
+@@
+
+  Tr *fn(...)
+  {
++   struct sn *nn = VTY_GET_CONTEXT(sn);
+  ...
+  \(
+-   struct sn *nn;
+    ...
+-   nn = vty->index;
++   if (!nn) {
++     return NULL;
++   }
+  \|
+-   struct sn *nn = vty->index;
++   if (!nn) {
++     return NULL;
++   }
+  \|
+-   struct sn *nn = vty->index;
+    ...
+-   nn = vty->index;
++   if (!nn) {
++     return NULL;
++   }
+  \)
+  ...
+  }
+
+/*
+ * variant 2:  vty wrapper func with (vty, vty->index, ...) signature
+ */
+
+/* find calls of this pattern first; arg will be dropped in rule3 */
+@rule1@
+identifier fn !~ "generic_(set|match)_";
+expression arg;
+@@
+
+ fn(arg, arg->index, ...)
+
+@ script:python @
+fn << rule1.fn;
+arg << rule1.arg;
+@@
+print "R01 removing vty-index argument on %s(%s, ...)" % (fn, arg)
+
+#/* strip arg on the vty wrapper func, add local handling */
+@ rule2 @
+identifier rule1.fn;
+identifier arg;
+identifier T;
+@@
+
+  static int fn (struct vty *vty,
+-                struct T * arg,
+                 ...)
+  {
++   VTY_DECLVAR_CONTEXT (T, arg);
+    ...
+  }
+
+/* drop argument on call sites identified earlier */
+@ rule3 @
+identifier rule1.fn;
+expression arg;
+@@
+
+  fn(arg,
+-    arg->index,
+     ...)
+
+
+/*
+ * variant 3:  function calls with "vty->index" argument (but no vty)
+ *
+ * a bit more complicated since we need to find the type from the header.
+ */
+
+/* find call sites first
+ * remember function name for later declvar insertion
+ */
+@ rule11 exists@
+identifier fn;
+identifier fparent;
+type Tr;
+@@
+
+  Tr fparent (...)
+  {
+    ...
+    fn(vty->index, ...)
+    ...
+  }
+
+@ script:python @
+fn << rule11.fn;
+@@
+print "R11 removing vty-index argument on %s(...)" % (fn)
+
+#/* find type of the argument - note args are mostly unnamed in Quagga :( */
+@ rule12 @
+identifier rule11.fn;
+identifier T, argname;
+type Tr;
+@@
+
+(
+  Tr fn(struct T *, ...);
+|
+  Tr fn(struct T * argname, ...);
+)
+
+@ script:python @
+fn << rule11.fn;
+T << rule12.T;
+@@
+print "R12 removing vty-index type is %s for %s(...)" % (T, fn)
+
+#/* add declvar
+# * this is split from rule14 so we support multiple calls in one func */
+@ rule13a @
+identifier rule11.fparent;
+identifier rule12.T;
+@@
+
+  int fparent (...)
+  {
++   VTY_DECLVAR_CONTEXT(T, T);
+    ...
+  }
+
+@ rule13b @
+identifier rule11.fparent;
+identifier rule12.T;
+type Tr;
+@@
+
+  Tr *fparent (...)
+  {
++   struct T *T = VTY_GET_CONTEXT(T);
++   if (!T) {
++     return NULL;
++   }
+    ...
+  }
+
+/* now replace the argument in the call */
+@ rule14 exists @
+identifier rule11.fn;
+identifier rule12.T;
+@@
+
+  {
+    ...
+    \(
+    fn(
+-       vty->index,
++       T,
+        ...)
+    \|
+    fn(
+-       vty->index
++       T
+        )
+    \)
+    ...
+  }
+
+/* special case ... */
+@rule30@
+identifier fn =~ "generic_(set|match)_";
+expression arg;
+@@
+
+  fn(arg,
+-    arg->index,
++    VTY_GET_CONTEXT(route_map_index),
+     ...)
+
+/* and finally - PUSH_CONTEXT */
+@ rule99a exists @
+identifier tnode;
+identifier vexpr =~ "NULL";
+@@
+
+- vty->node = tnode;
+  ...
+- vty->index = vexpr;
++ VTY_PUSH_CONTEXT_NULL(tnode);
+
+@ rule99b exists @
+identifier tnode;
+expression vexpr;
+@@
+
+- vty->node = tnode;
+  ...
+- vty->index = vexpr;
++ VTY_PUSH_CONTEXT(tnode, vexpr);
+
diff --git a/tools/vty_index.sh b/tools/vty_index.sh
@@ -0,0 +1,21 @@
+#!/bin/sh
+
+sed -e '1istatic void bgp_debug_clear_updgrp_update_dbg(struct bgp *bgp);' -i \
+	bgpd/bgp_debug.c
+sed -e 's%^#if 0%#if 1 /* 0 */%' -i \
+	ospfd/ospf_vty.c \
+	ospf6d/ospf6_top.c \
+	#
+spatch \
+	--sp-file    tools/vty_index.cocci \
+	--macro-file tools/cocci.h \
+	--all-includes -I . -I lib \
+	--use-gitgrep --dir . --in-place
+
+sed -e 's%^#if 1 /\* 0 \*/%#if 0%' -i \
+	ospfd/ospf_vty.c \
+	ospf6d/ospf6_top.c \
+	#
+sed -e '1d' -i \
+	bgpd/bgp_debug.c
+