From a07665b7654eb87d6358c6af3f45dce5a6daee75 Mon Sep 17 00:00:00 2001
From: Nischal Sheth <nsheth@juniper.net>
Date: Mon, 27 Mar 2017 11:55:51 -0700
Subject: [PATCH] Use add/delete/get methods when processing SubscriptionState

Change-Id: I82028b85453fafb67bebca7d28a30c3ad44efa75
Partial-Bug: 1672512
---
 src/bgp/bgp_xmpp_channel.cc | 43 +++++++++++++++++++++++++------------
 src/bgp/bgp_xmpp_channel.h  |  5 +++++
 2 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/src/bgp/bgp_xmpp_channel.cc b/src/bgp/bgp_xmpp_channel.cc
index f1cc2e9cfc9..4683e3e7642 100644
--- a/src/bgp/bgp_xmpp_channel.cc
+++ b/src/bgp/bgp_xmpp_channel.cc
@@ -578,11 +578,11 @@ void BgpXmppChannel::RoutingInstanceCallback(string vrf_name, int op) {
         ProcessDeferredSubscribeRequest(rt_instance, imr_state);
         DeleteInstanceMembershipState(vrf_name);
     } else {
-        SubscribedRoutingInstanceList::iterator it =
-            routing_instances_.find(rt_instance);
-        if (it == routing_instances_.end()) return;
-        rtarget_manager_->RoutingInstanceCallback(it->first,
-                                                           &it->second.targets);
+        SubscriptionState *sub_state = NULL;
+        if (!GetSubscriptionState(rt_instance, &sub_state))
+            return;
+        rtarget_manager_->RoutingInstanceCallback(
+            rt_instance, &sub_state->targets);
     }
 }
 
@@ -2212,6 +2212,25 @@ void BgpXmppChannel::AddSubscriptionState(RoutingInstance *rt_instance,
     }
 }
 
+void BgpXmppChannel::DeleteSubscriptionState(RoutingInstance *rt_instance) {
+    routing_instances_.erase(rt_instance);
+}
+
+bool BgpXmppChannel::GetSubscriptionState(RoutingInstance *rt_instance,
+    SubscriptionState **sub_state) {
+    SubscribedRoutingInstanceList::iterator loc =
+        routing_instances_.find(rt_instance);
+    if (loc != routing_instances_.end()) {
+        if (sub_state)
+            *sub_state = &loc->second;
+        return true;
+    } else {
+        if (sub_state)
+            *sub_state = NULL;
+        return false;
+    }
+}
+
 void BgpXmppChannel::ProcessDeferredSubscribeRequest(RoutingInstance *instance,
     const InstanceMembershipRequestState &imr_state) {
     int instance_id = imr_state.instance_id;
@@ -2298,8 +2317,7 @@ void BgpXmppChannel::ProcessSubscriptionRequest(
                              "Duplicate subscribe for routing instance " <<
                              vrf_name << ", triggering close");
                 channel_->Close();
-            } else if (routing_instances_.find(rt_instance) !=
-                routing_instances_.end()) {
+            } else if (GetSubscriptionState(rt_instance)) {
                 BGP_LOG_PEER_WARNING(Membership, Peer(),
                              BGP_LOG_FLAG_ALL, BGP_PEER_DIR_NA,
                              "Duplicate subscribe for routing instance " <<
@@ -2320,8 +2338,7 @@ void BgpXmppChannel::ProcessSubscriptionRequest(
                 FlushDeferQ(vrf_name);
                 channel_stats_.instance_unsubscribe++;
                 return;
-            } else if (routing_instances_.find(rt_instance) ==
-                routing_instances_.end()) {
+            } else if (!GetSubscriptionState(rt_instance)) {
                 BGP_LOG_PEER_WARNING(Membership, Peer(),
                              BGP_LOG_FLAG_ALL, BGP_PEER_DIR_NA,
                              "Spurious unsubscribe for routing instance " <<
@@ -2333,8 +2350,7 @@ void BgpXmppChannel::ProcessSubscriptionRequest(
         }
     } else {
         if (add_change) {
-            if (routing_instances_.find(rt_instance) !=
-                routing_instances_.end()) {
+            if (GetSubscriptionState(rt_instance)) {
                 if (!close_manager_->IsCloseInProgress()) {
                     BGP_LOG_PEER_WARNING(Membership, Peer(),
                                  BGP_LOG_FLAG_ALL, BGP_PEER_DIR_NA,
@@ -2346,8 +2362,7 @@ void BgpXmppChannel::ProcessSubscriptionRequest(
             }
             channel_stats_.instance_subscribe++;
         } else {
-            if (routing_instances_.find(rt_instance) ==
-                routing_instances_.end()) {
+            if (!GetSubscriptionState(rt_instance)) {
                 BGP_LOG_PEER_WARNING(Membership, Peer(),
                              BGP_LOG_FLAG_ALL, BGP_PEER_DIR_NA,
                              "Spurious unsubscribe for routing instance " <<
@@ -2363,7 +2378,7 @@ void BgpXmppChannel::ProcessSubscriptionRequest(
         AddSubscriptionState(rt_instance, instance_id);
     } else  {
         rtarget_manager_->PublishRTargetRoute(rt_instance, false);
-        routing_instances_.erase(rt_instance);
+        DeleteSubscriptionState(rt_instance);
     }
 
     RoutingInstance::RouteTableList const rt_list = rt_instance->GetTables();
diff --git a/src/bgp/bgp_xmpp_channel.h b/src/bgp/bgp_xmpp_channel.h
index df29810708e..b5ea4ba2fb3 100644
--- a/src/bgp/bgp_xmpp_channel.h
+++ b/src/bgp/bgp_xmpp_channel.h
@@ -232,6 +232,8 @@ class BgpXmppChannel {
             GR_STALE = 1 << 0,
             LLGR_STALE = 1 << 1
         };
+
+        SubscriptionState() : index(-1), state(NONE) { }
         SubscriptionState(const RoutingInstance::RouteTargetList &targets,
                           int index)
                 : targets(targets), index(index), state(NONE) { }
@@ -301,6 +303,9 @@ class BgpXmppChannel {
                                     const XmppStanza::XmppMessageIq *iq,
                                     bool add_change);
     void AddSubscriptionState(RoutingInstance *rt_instance, int index);
+    void DeleteSubscriptionState(RoutingInstance *rt_instance);
+    bool GetSubscriptionState(RoutingInstance *rt_instance,
+        SubscriptionState **sub_state = NULL);
 
     void RegisterTable(int line, BgpTable *table,
         const TableMembershipRequestState *tmr_state);