Cleanup signatures of a couple of methods in bgp_xmpp_channel.cc

Change-Id: Iae5f81f929702040c17f7c62dcb6c83858e71ebb
Partial-Bug: 1672512

src/bgp/bgp_xmpp_channel.cc

@@ -572,14 +572,15 @@ void BgpXmppChannel::RoutingInstanceCallback(string vrf_name, int op) {
     assert(rt_instance);
 
     if (op == RoutingInstanceMgr::INSTANCE_ADD) {
-        InstanceMembershipRequestState imr_state;
-        if (!GetInstanceMembershipState(vrf_name, &imr_state))
+        const InstanceMembershipRequestState *imr_state =
+            GetInstanceMembershipState(vrf_name);
+        if (!imr_state)
             return;
-        ProcessDeferredSubscribeRequest(rt_instance, imr_state);
+        ProcessDeferredSubscribeRequest(rt_instance, *imr_state);
         DeleteInstanceMembershipState(vrf_name);
     } else {
-        SubscriptionState *sub_state = NULL;
-        if (!GetSubscriptionState(rt_instance, &sub_state))
+        SubscriptionState *sub_state = GetSubscriptionState(rt_instance);
+        if (!sub_state)
             return;
         rtarget_manager_->RoutingInstanceCallback(
             rt_instance, &sub_state->targets);
@@ -689,23 +690,13 @@ bool BgpXmppChannel::DeleteInstanceMembershipState(const string &instance) {
 }
 
 //
-// Return true if there's an entry in the pending instance request map.
+// Find the entry in the pending instance request map.
 //
-bool BgpXmppChannel::GetInstanceMembershipState(const string &instance,
-    InstanceMembershipRequestState *imr_state) const {
+const BgpXmppChannel::InstanceMembershipRequestState *
+BgpXmppChannel::GetInstanceMembershipState(const string &instance) const {
     InstanceMembershipRequestMap::const_iterator loc =
         instance_membership_request_map_.find(instance);
-    if (loc != instance_membership_request_map_.end()) {
-        if (imr_state) {
-            *imr_state = loc->second;
-        }
-        return true;
-    } else {
-        if (imr_state) {
-            imr_state->instance_id = -1;
-        }
-        return false;
-    }
+    return loc != instance_membership_request_map_.end() ? &loc->second : NULL;
 }
 
 //
@@ -753,9 +744,10 @@ bool BgpXmppChannel::VerifyMembership(const string &vrf_name,
         // Bail if there's no pending subscribe for the instance.
         // Note that route retract can be received while the instance is
         // marked for deletion.
-        InstanceMembershipRequestState imr_state;
-        if (GetInstanceMembershipState(vrf_name, &imr_state)) {
-            *instance_id = imr_state.instance_id;
+        const InstanceMembershipRequestState *imr_state =
+            GetInstanceMembershipState(vrf_name);
+        if (imr_state) {
+            *instance_id = imr_state->instance_id;
             *subscribe_pending = true;
         } else if (add_change || !rt_instance) {
             BGP_LOG_PEER_INSTANCE_CRITICAL(Peer(), vrf_name, BGP_PEER_DIR_IN,
@@ -2216,19 +2208,18 @@ void BgpXmppChannel::DeleteSubscriptionState(RoutingInstance *rt_instance) {
     routing_instances_.erase(rt_instance);
 }
 
-bool BgpXmppChannel::GetSubscriptionState(RoutingInstance *rt_instance,
-    SubscriptionState **sub_state) {
+BgpXmppChannel::SubscriptionState *BgpXmppChannel::GetSubscriptionState(
+    RoutingInstance *rt_instance) {
     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;
-    }
+    return (loc != routing_instances_.end() ? &loc->second : NULL);
+}
+
+const BgpXmppChannel::SubscriptionState *BgpXmppChannel::GetSubscriptionState(
+    RoutingInstance *rt_instance) const {
+    SubscribedRoutingInstanceList::const_iterator loc =
+        routing_instances_.find(rt_instance);
+    return (loc != routing_instances_.end() ? &loc->second : NULL);
 }
 
 void BgpXmppChannel::ProcessDeferredSubscribeRequest(RoutingInstance *instance,

src/bgp/bgp_xmpp_channel.h

@@ -181,6 +181,16 @@ class BgpXmppChannel {
     void ProcessPendingSubscriptions();
 
 protected:
+    struct InstanceMembershipRequestState {
+        InstanceMembershipRequestState(int instance_id = -1,
+            bool no_ribout = false)
+            : instance_id(instance_id), no_ribout(no_ribout) {
+        };
+
+        int instance_id;
+        bool no_ribout;
+    };
+
     XmppChannel *channel_;
 
 private:
@@ -215,16 +225,6 @@ class BgpXmppChannel {
         bool no_ribout;
     };
 
-    struct InstanceMembershipRequestState {
-        InstanceMembershipRequestState(int instance_id = -1,
-            bool no_ribout = false)
-            : instance_id(instance_id), no_ribout(no_ribout) {
-        };
-
-        int instance_id;
-        bool no_ribout;
-    };
-
     // Map of routing instances to which this BgpXmppChannel is subscribed.
     struct SubscriptionState {
         enum State {
@@ -250,6 +250,7 @@ class BgpXmppChannel {
         int index;
         uint32_t state;
     };
+
     typedef std::map<RoutingInstance *, SubscriptionState>
         SubscribedRoutingInstanceList;
 
@@ -288,8 +289,8 @@ class BgpXmppChannel {
     void AddInstanceMembershipState(const std::string &instance,
         InstanceMembershipRequestState imr_state);
     bool DeleteInstanceMembershipState(const std::string &instance);
-    virtual bool GetInstanceMembershipState(const std::string &instance,
-        InstanceMembershipRequestState *imr_state = NULL) const;
+    virtual const InstanceMembershipRequestState *GetInstanceMembershipState(
+        const std::string &instance) const;
 
     bool ProcessItem(std::string vrf_name, const pugi::xml_node &node,
                      bool add_change);
@@ -304,8 +305,9 @@ class BgpXmppChannel {
                                     bool add_change);
     void AddSubscriptionState(RoutingInstance *rt_instance, int index);
     void DeleteSubscriptionState(RoutingInstance *rt_instance);
-    bool GetSubscriptionState(RoutingInstance *rt_instance,
-        SubscriptionState **sub_state = NULL);
+    SubscriptionState *GetSubscriptionState(RoutingInstance *rt_instance);
+    const SubscriptionState *GetSubscriptionState(
+        RoutingInstance *rt_instance) const;
 
     void RegisterTable(int line, BgpTable *table,
         const TableMembershipRequestState *tmr_state);

src/bgp/test/bgp_xmpp_parse_test.cc

@@ -96,19 +96,20 @@ class XmppChannelMock : public XmppChannel {
 class BgpXmppChannelMock : public BgpXmppChannel {
 public:
     BgpXmppChannelMock(XmppChannel *channel, BgpServer *server)
-        : BgpXmppChannel(channel, server) {
+        : BgpXmppChannel(channel, server), imr_state_(1) {
     }
 
     virtual boost::asio::ip::tcp::endpoint endpoint() const {
         return boost::asio::ip::tcp::endpoint();
     }
 
 private:
-    virtual bool GetInstanceMembershipState(const string &vrf_name,
-        InstanceMembershipRequestState *imr_state) {
-        imr_state->instance_id = 1;
-        return true;
+    virtual const InstanceMembershipRequestState *GetInstanceMembershipState(
+        const string &vrf_name) const {
+        return &imr_state_;
     }
+
+    InstanceMembershipRequestState imr_state_;
 };
 
 class BgpXmppParseTest : public ::testing::Test {