Acked-by: Donald Sharp <[email protected]>

On Wed, Feb 24, 2016 at 12:26 AM, David Lamparter <
[email protected]> wrote:

> bgpd, ospf6d, isisd and some tests were reusing MTYPEs defined in the
> library for its own use.  This is bad practice and will break with the
> later commit making the library's MTYPEs static.
>
> Signed-off-by: David Lamparter <[email protected]>
> ---
>  bgpd/bgp_nexthop.c       |  8 +++++---
>  isisd/isis_memory.c      |  2 ++
>  isisd/isis_memory.h      |  2 ++
>  isisd/isis_redist.c      | 13 +++++++------
>  ospf6d/ospf6_interface.c | 10 ++++++----
>  tests/heavy-wq.c         | 14 +++++++++-----
>  tests/test-memory.c      | 49
> +++++++++++++++++++++++++-----------------------
>  7 files changed, 57 insertions(+), 41 deletions(-)
>
> diff --git a/bgpd/bgp_nexthop.c b/bgpd/bgp_nexthop.c
> index 4375be9..158f1df 100644
> --- a/bgpd/bgp_nexthop.c
> +++ b/bgpd/bgp_nexthop.c
> @@ -42,6 +42,8 @@ Software Foundation, Inc., 59 Temple Place - Suite 330,
> Boston, MA
>  #include "zebra/rib.h"
>  #include "zebra/zserv.h"       /* For ZEBRA_SERV_PATH. */
>
> +DEFINE_MTYPE_STATIC(BGPD, BGP_NEXTHOP, "BGP nexthop")
> +
>  struct bgp_nexthop_cache *zlookup_query (struct in_addr);
>  struct bgp_nexthop_cache *zlookup_query_ipv6 (struct in6_addr *);
>
> @@ -92,7 +94,7 @@ bnc_nexthop_free (struct bgp_nexthop_cache *bnc)
>    for (nexthop = bnc->nexthop; nexthop; nexthop = next)
>      {
>        next = nexthop->next;
> -      XFREE (MTYPE_NEXTHOP, nexthop);
> +      XFREE (MTYPE_BGP_NEXTHOP, nexthop);
>      }
>  }
>
> @@ -819,7 +821,7 @@ zlookup_read (void)
>
>        for (i = 0; i < nexthop_num; i++)
>         {
> -         nexthop = XCALLOC (MTYPE_NEXTHOP, sizeof (struct nexthop));
> +         nexthop = XCALLOC (MTYPE_BGP_NEXTHOP, sizeof (struct nexthop));
>           nexthop->type = stream_getc (s);
>           switch (nexthop->type)
>             {
> @@ -922,7 +924,7 @@ zlookup_read_ipv6 (void)
>
>        for (i = 0; i < nexthop_num; i++)
>         {
> -         nexthop = XCALLOC (MTYPE_NEXTHOP, sizeof (struct nexthop));
> +         nexthop = XCALLOC (MTYPE_BGP_NEXTHOP, sizeof (struct nexthop));
>           nexthop->type = stream_getc (s);
>           switch (nexthop->type)
>             {
> diff --git a/isisd/isis_memory.c b/isisd/isis_memory.c
> index 9bc506e..5c5fee7 100644
> --- a/isisd/isis_memory.c
> +++ b/isisd/isis_memory.c
> @@ -43,3 +43,5 @@ DEFINE_MTYPE(ISISD, ISIS_NEXTHOP,       "ISIS nexthop")
>  DEFINE_MTYPE(ISISD, ISIS_NEXTHOP6,      "ISIS nexthop6")
>  DEFINE_MTYPE(ISISD, ISIS_DICT,          "ISIS dictionary")
>  DEFINE_MTYPE(ISISD, ISIS_DICT_NODE,     "ISIS dictionary node")
> +DEFINE_MTYPE(ISISD, ISIS_EXT_ROUTE,     "ISIS redistributed route")
> +DEFINE_MTYPE(ISISD, ISIS_EXT_INFO,      "ISIS redistributed route info")
> diff --git a/isisd/isis_memory.h b/isisd/isis_memory.h
> index 40bb88a..6ebce1f 100644
> --- a/isisd/isis_memory.h
> +++ b/isisd/isis_memory.h
> @@ -42,5 +42,7 @@ DECLARE_MTYPE(ISIS_NEXTHOP)
>  DECLARE_MTYPE(ISIS_NEXTHOP6)
>  DECLARE_MTYPE(ISIS_DICT)
>  DECLARE_MTYPE(ISIS_DICT_NODE)
> +DECLARE_MTYPE(ISIS_EXT_ROUTE)
> +DECLARE_MTYPE(ISIS_EXT_INFO)
>
>  #endif /* _QUAGGA_ISIS_MEMORY_H */
> diff --git a/isisd/isis_redist.c b/isisd/isis_redist.c
> index 4a84d51..dc82678 100644
> --- a/isisd/isis_redist.c
> +++ b/isisd/isis_redist.c
> @@ -24,6 +24,7 @@
>  #include "if.h"
>  #include "linklist.h"
>  #include "memory.h"
> +#include "isis_memory.h"
>  #include "prefix.h"
>  #include "routemap.h"
>  #include "stream.h"
> @@ -95,7 +96,7 @@ isis_redist_route_node_create(route_table_delegate_t
> *delegate,
>                                struct route_table *table)
>  {
>    struct route_node *node;
> -  node = XCALLOC(MTYPE_ROUTE_NODE, sizeof(*node));
> +  node = XCALLOC(MTYPE_ISIS_EXT_ROUTE, sizeof(*node));
>    return node;
>  }
>
> @@ -105,8 +106,8 @@ isis_redist_route_node_destroy(route_table_delegate_t
> *delegate,
>                                 struct route_node *node)
>  {
>    if (node->info)
> -    XFREE(MTYPE_ISIS, node->info);
> -  XFREE (MTYPE_ROUTE_NODE, node);
> +    XFREE(MTYPE_ISIS_EXT_INFO, node->info);
> +  XFREE (MTYPE_ISIS_EXT_ROUTE, node);
>  }
>
>  static route_table_delegate_t isis_redist_rt_delegate = {
> @@ -143,7 +144,7 @@ isis_redist_install(struct isis_area *area, int level,
>      }
>    else
>      {
> -      er_node->info = XMALLOC(MTYPE_ISIS, sizeof(*info));
> +      er_node->info = XMALLOC(MTYPE_ISIS_EXT_INFO, sizeof(*info));
>      }
>
>    memcpy(er_node->info, info, sizeof(*info));
> @@ -242,7 +243,7 @@ isis_redist_ensure_default(struct isis *isis, int
> family)
>        return;
>      }
>
> -  ei_node->info = XCALLOC(MTYPE_ISIS, sizeof(struct isis_ext_info));
> +  ei_node->info = XCALLOC(MTYPE_ISIS_EXT_INFO, sizeof(struct
> isis_ext_info));
>
>    info = ei_node->info;
>    info->origin = DEFAULT_ROUTE;
> @@ -280,7 +281,7 @@ isis_redist_add(int type, struct prefix *p, u_char
> distance, uint32_t metric)
>    if (ei_node->info)
>      route_unlock_node(ei_node);
>    else
> -    ei_node->info = XCALLOC(MTYPE_ISIS, sizeof(struct isis_ext_info));
> +    ei_node->info = XCALLOC(MTYPE_ISIS_EXT_INFO, sizeof(struct
> isis_ext_info));
>
>    info = ei_node->info;
>    info->origin = type;
> diff --git a/ospf6d/ospf6_interface.c b/ospf6d/ospf6_interface.c
> index 26f68ac..d03ae76 100644
> --- a/ospf6d/ospf6_interface.c
> +++ b/ospf6d/ospf6_interface.c
> @@ -43,6 +43,8 @@
>  #include "ospf6_snmp.h"
>  #include "ospf6d.h"
>
> +DEFINE_MTYPE_STATIC(OSPF6D, CFG_PLIST_NAME, "configured prefix list
> names")
> +
>  unsigned char conf_debug_ospf6_interface = 0;
>
>  const char *ospf6_interface_state_str[] =
> @@ -259,7 +261,7 @@ ospf6_interface_delete (struct ospf6_interface *oi)
>
>    /* plist_name */
>    if (oi->plist_name)
> -    XFREE (MTYPE_PREFIX_LIST_STR, oi->plist_name);
> +    XFREE (MTYPE_CFG_PLIST_NAME, oi->plist_name);
>
>    XFREE (MTYPE_OSPF6_IF, oi);
>  }
> @@ -1642,8 +1644,8 @@ DEFUN (ipv6_ospf6_advertise_prefix_list,
>    assert (oi);
>
>    if (oi->plist_name)
> -    XFREE (MTYPE_PREFIX_LIST_STR, oi->plist_name);
> -  oi->plist_name = XSTRDUP (MTYPE_PREFIX_LIST_STR, argv[0]);
> +    XFREE (MTYPE_CFG_PLIST_NAME, oi->plist_name);
> +  oi->plist_name = XSTRDUP (MTYPE_CFG_PLIST_NAME, argv[0]);
>
>    ospf6_interface_connected_route_update (oi->interface);
>
> @@ -1684,7 +1686,7 @@ DEFUN (no_ipv6_ospf6_advertise_prefix_list,
>
>    if (oi->plist_name)
>      {
> -      XFREE (MTYPE_PREFIX_LIST_STR, oi->plist_name);
> +      XFREE (MTYPE_CFG_PLIST_NAME, oi->plist_name);
>        oi->plist_name = NULL;
>      }
>
> diff --git a/tests/heavy-wq.c b/tests/heavy-wq.c
> index 2f133cc..2d15dc3 100644
> --- a/tests/heavy-wq.c
> +++ b/tests/heavy-wq.c
> @@ -38,6 +38,10 @@
>
>  #include "tests.h"
>
> +DEFINE_MGROUP(TEST_HEAVYWQ, "heavy-wq test")
> +DEFINE_MTYPE_STATIC(TEST_HEAVYWQ, WQ_NODE, "heavy_wq_node")
> +DEFINE_MTYPE_STATIC(TEST_HEAVYWQ, WQ_NODE_STR, "heavy_wq_node->str")
> +
>  extern struct thread_master *master;
>  static struct work_queue *heavy_wq;
>
> @@ -61,17 +65,17 @@ heavy_wq_add (struct vty *vty, const char *str, int i)
>  {
>    struct heavy_wq_node *hn;
>
> -  if ((hn = XCALLOC (MTYPE_PREFIX_LIST, sizeof(struct heavy_wq_node))) ==
> NULL)
> +  if ((hn = XCALLOC (MTYPE_WQ_NODE, sizeof(struct heavy_wq_node))) ==
> NULL)
>      {
>        zlog_err ("%s: unable to allocate hn", __func__);
>        return;
>      }
>
>    hn->i = i;
> -  if (!(hn->str = XSTRDUP (MTYPE_PREFIX_LIST_STR, str)))
> +  if (!(hn->str = XSTRDUP (MTYPE_WQ_NODE_STR, str)))
>      {
>        zlog_err ("%s: unable to xstrdup", __func__);
> -      XFREE (MTYPE_PREFIX_LIST, hn);
> +      XFREE (MTYPE_WQ_NODE, hn);
>        return;
>      }
>
> @@ -92,9 +96,9 @@ slow_func_del (struct work_queue *wq, void *data)
>    struct heavy_wq_node *hn = data;
>    assert (hn && hn->str);
>    printf ("%s: %s\n", __func__, hn->str);
> -  XFREE (MTYPE_PREFIX_LIST_STR, hn->str);
> +  XFREE (MTYPE_WQ_NODE_STR, hn->str);
>    hn->str = NULL;
> -  XFREE(MTYPE_PREFIX_LIST, hn);
> +  XFREE(MTYPE_WQ_NODE, hn);
>  }
>
>  static wq_item_status
> diff --git a/tests/test-memory.c b/tests/test-memory.c
> index 807249e..6849b9d 100644
> --- a/tests/test-memory.c
> +++ b/tests/test-memory.c
> @@ -20,6 +20,9 @@
>  #include <zebra.h>
>  #include <memory.h>
>
> +DEFINE_MGROUP(TEST_MEMORY, "memory test")
> +DEFINE_MTYPE_STATIC(TEST_MEMORY, TEST, "generic test mtype")
> +
>  /* Memory torture tests
>   *
>   * Tests below are generic but comments are focused on interaction with
> @@ -52,28 +55,28 @@ main(int argc, char **argv)
>    /* simple case, test cache */
>    for (i = 0; i < TIMES; i++)
>      {
> -      a[0] = XMALLOC (MTYPE_VTY, 1024);
> +      a[0] = XMALLOC (MTYPE_TEST, 1024);
>        memset (a[0], 1, 1024);
> -      a[1] = XMALLOC (MTYPE_VTY, 1024);
> +      a[1] = XMALLOC (MTYPE_TEST, 1024);
>        memset (a[1], 1, 1024);
> -      XFREE(MTYPE_VTY, a[0]); /* should go to cache */
> -      a[0] = XMALLOC (MTYPE_VTY, 1024); /* should be satisfied from cache
> */
> -      XFREE(MTYPE_VTY, a[0]);
> -      XFREE(MTYPE_VTY, a[1]);
> +      XFREE(MTYPE_TEST, a[0]); /* should go to cache */
> +      a[0] = XMALLOC (MTYPE_TEST, 1024); /* should be satisfied from
> cache */
> +      XFREE(MTYPE_TEST, a[0]);
> +      XFREE(MTYPE_TEST, a[1]);
>      }
>
>    printf ("malloc x, malloc y, free x, malloc y, free free\n\n");
>    /* cache should go invalid, valid, invalid, etc.. */
>    for (i = 0; i < TIMES; i++)
>      {
> -      a[0] = XMALLOC (MTYPE_VTY, 512);
> +      a[0] = XMALLOC (MTYPE_TEST, 512);
>        memset (a[0], 1, 512);
> -      a[1] = XMALLOC (MTYPE_VTY, 1024); /* invalidate cache */
> +      a[1] = XMALLOC (MTYPE_TEST, 1024); /* invalidate cache */
>        memset (a[1], 1, 1024);
> -      XFREE(MTYPE_VTY, a[0]);
> -      a[0] = XMALLOC (MTYPE_VTY, 1024);
> -      XFREE(MTYPE_VTY, a[0]);
> -      XFREE(MTYPE_VTY, a[1]);
> +      XFREE(MTYPE_TEST, a[0]);
> +      a[0] = XMALLOC (MTYPE_TEST, 1024);
> +      XFREE(MTYPE_TEST, a[0]);
> +      XFREE(MTYPE_TEST, a[1]);
>        /* cache should become valid again on next request */
>      }
>
> @@ -81,12 +84,12 @@ main(int argc, char **argv)
>    /* test calloc */
>    for (i = 0; i < TIMES; i++)
>      {
> -      a[0] = XCALLOC (MTYPE_VTY, 1024);
> +      a[0] = XCALLOC (MTYPE_TEST, 1024);
>        memset (a[0], 1, 1024);
> -      a[1] = XCALLOC (MTYPE_VTY, 512); /* invalidate cache */
> +      a[1] = XCALLOC (MTYPE_TEST, 512); /* invalidate cache */
>        memset (a[1], 1, 512);
> -      XFREE(MTYPE_VTY, a[1]);
> -      XFREE(MTYPE_VTY, a[0]);
> +      XFREE(MTYPE_TEST, a[1]);
> +      XFREE(MTYPE_TEST, a[0]);
>        /* alloc == 0, cache can become valid again on next request */
>      }
>
> @@ -95,27 +98,27 @@ main(int argc, char **argv)
>    for (i = 0; i < TIMES; i++)
>      {
>        printf ("calloc a0 1024\n");
> -      a[0] = XCALLOC (MTYPE_VTY, 1024);
> +      a[0] = XCALLOC (MTYPE_TEST, 1024);
>        memset (a[0], 1, 1024/2);
>
>        printf ("calloc 1 1024\n");
> -      a[1] = XCALLOC (MTYPE_VTY, 1024);
> +      a[1] = XCALLOC (MTYPE_TEST, 1024);
>        memset (a[1], 1, 1024/2);
>
>        printf ("realloc 0 1024\n");
> -      a[3] = XREALLOC (MTYPE_VTY, a[0], 2048); /* invalidate cache */
> +      a[3] = XREALLOC (MTYPE_TEST, a[0], 2048); /* invalidate cache */
>        if (a[3] != NULL)
>          a[0] = a[3];
>        memset (a[0], 1, 1024);
>
>        printf ("calloc 2 512\n");
> -      a[2] = XCALLOC (MTYPE_VTY, 512);
> +      a[2] = XCALLOC (MTYPE_TEST, 512);
>        memset (a[2], 1, 512);
>
>        printf ("free 1 0 2\n");
> -      XFREE(MTYPE_VTY, a[1]);
> -      XFREE(MTYPE_VTY, a[0]);
> -      XFREE(MTYPE_VTY, a[2]);
> +      XFREE(MTYPE_TEST, a[1]);
> +      XFREE(MTYPE_TEST, a[0]);
> +      XFREE(MTYPE_TEST, a[2]);
>        /* alloc == 0, cache valid next request */
>      }
>    return 0;
> --
> 2.3.6
>
>
> _______________________________________________
> Quagga-dev mailing list
> [email protected]
> https://lists.quagga.net/mailman/listinfo/quagga-dev
>
_______________________________________________
Quagga-dev mailing list
[email protected]
https://lists.quagga.net/mailman/listinfo/quagga-dev

Reply via email to