From 36dbb3764579550c2fbc73605923024a5eef5ccf Mon Sep 17 00:00:00 2001
From: Teemu Ollakka <teemu.ollakka@galeracluster.com>
Date: Sat, 15 Dec 2018 13:46:42 +0000
Subject: [PATCH] codership/wsrep-lib#35 Rewrote member_index() to use
 iterators

Use iterators for scanning members vector in order to avoid
issues with integer signedness and range checks. The vector is
usually rather small and not in hot codepath, so performance
is here not an issue.

Added unit test for member_index() method.
---
 include/wsrep/view.hpp |  5 ++++-
 src/view.cpp           | 15 ++++-----------
 test/CMakeLists.txt    |  1 +
 test/view_test.cpp     | 41 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 50 insertions(+), 12 deletions(-)
 create mode 100644 test/view_test.cpp

diff --git a/include/wsrep/view.hpp b/include/wsrep/view.hpp
index ecac3ee..9913db5 100644
--- a/include/wsrep/view.hpp
+++ b/include/wsrep/view.hpp
@@ -116,7 +116,10 @@ namespace wsrep
         }
 
         /**
-         * Return member index in the view
+         * Return member index in the view.
+         *
+         * @return Member index if found, -1 if member is not present
+         *         in the view.
          */
         int member_index(const wsrep::id& member_id) const;
 
diff --git a/src/view.cpp b/src/view.cpp
index 0feb81f..9bea0e6 100644
--- a/src/view.cpp
+++ b/src/view.cpp
@@ -22,21 +22,14 @@
 
 int wsrep::view::member_index(const wsrep::id& member_id) const
 {
-    // first, quick guess
-    if (own_index_ >= 0 && members_[own_index_].id() == member_id)
+    for (std::vector<member>::const_iterator i(members_.begin());
+         i != members_.end(); ++i)
     {
-        return own_index_;
-    }
-
-    // guesing didn't work, scan the list
-    for (size_t i(0); i < members_.size(); ++i)
-    {
-        if (static_cast<int>(i) != own_index_ && members_[i].id() == member_id)
+        if (i->id() == member_id)
         {
-            return i;
+            return (i - members_.begin());
         }
     }
-
     return -1;
 }
 
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index 5bc50c2..bc715a4 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -11,6 +11,7 @@ add_executable(wsrep-lib_test
   server_context_test.cpp
   transaction_test.cpp
   transaction_test_2pc.cpp
+  view_test.cpp
   wsrep-lib_test.cpp
   )
 
diff --git a/test/view_test.cpp b/test/view_test.cpp
new file mode 100644
index 0000000..2caa51e
--- /dev/null
+++ b/test/view_test.cpp
@@ -0,0 +1,41 @@
+/*
+ * Copyright (C) 2018 Codership Oy <info@codership.com>
+ *
+ * This file is part of wsrep-lib.
+ *
+ * Wsrep-lib is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * Wsrep-lib is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with wsrep-lib.  If not, see <https://www.gnu.org/licenses/>.
+ */
+
+#include "wsrep/view.hpp"
+#include <boost/test/unit_test.hpp>
+
+BOOST_AUTO_TEST_CASE(view_test_member_index)
+{
+    std::vector<wsrep::view::member> members;
+    members.push_back(wsrep::view::member(wsrep::id("1"), "", ""));
+    members.push_back(wsrep::view::member(wsrep::id("2"), "", ""));
+    members.push_back(wsrep::view::member(wsrep::id("3"), "", ""));
+
+    wsrep::view view(wsrep::gtid(wsrep::id("cluster"), wsrep::seqno(1)),
+		     wsrep::seqno(1),
+		     wsrep::view::primary,
+		     0,
+		     1,
+		     0,
+		     members);
+    BOOST_REQUIRE(view.member_index(wsrep::id("1")) == 0);
+    BOOST_REQUIRE(view.member_index(wsrep::id("2")) == 1);
+    BOOST_REQUIRE(view.member_index(wsrep::id("3")) == 2);
+    BOOST_REQUIRE(view.member_index(wsrep::id("4")) == -1);
+}