rgw-multisite: metadata conflict not computed correctly

The former logic with a conditional based on `++i == 0` would never
execute. So this uses a boolean to differentiate the first from other
iterations and tries to clarify the code through commenting and an
explicit declaration. Additionally a warning is eliminated by
initializing a variable.

Signed-off-by: J. Eric Ivancich <ivancich@redhat.com>

diff --git a/src/rgw/rgw_bucket_sync.cc b/src/rgw/rgw_bucket_sync.cc
index 045d6bdf4b5b616bb61b1c6b4c5333f159c4097c..5fd81c53b1e7658e6260d40b2a7be54326f763d1 100644 (file)
--- a/src/rgw/rgw_bucket_sync.cc
+++ b/src/rgw/rgw_bucket_sync.cc
@@ -1,4 +1,5 @@
-
+// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
+// vim: ts=8 sw=2 smarttab ft=cpp
 
 #include "rgw_common.h"
 #include "rgw_bucket_sync.h"
@@ -350,31 +351,36 @@ bool RGWBucketSyncFlowManager::pipe_rules::find_basic_info_without_tags(const rg
     return false;
   }
 
-  bool conflict = false;
-
   std::optional<rgw_user> _user;
   std::optional<rgw_sync_pipe_acl_translation> _acl_translation;
   std::optional<string> _storage_class;
   rgw_sync_pipe_params::Mode _mode{rgw_sync_pipe_params::Mode::MODE_SYSTEM};
 
-  int i = 0;
+  // make sure all params are the same by saving the first one
+  // encountered and comparing all subsequent to it
+  bool first_iter = true;
   for (auto& iter : iters) {
-    auto& rule_params = iter->second->params;
-    if (++i == 0) {
+    const rgw_sync_pipe_params& rule_params = iter->second->params;
+    if (first_iter) {
       _user = rule_params.user;
       _acl_translation = rule_params.dest.acl_translation;
       _storage_class = rule_params.dest.storage_class;
       _mode = rule_params.mode;
-      continue;
-    }
-
-    conflict = !(_user == rule_params.user &&
-                 _acl_translation == rule_params.dest.acl_translation &&
-                 _storage_class == rule_params.dest.storage_class &&
-                 _mode == rule_params.mode);
-    if (conflict) {
-      *need_more_info = true;
-      return false;
+      first_iter = false;
+    } else {
+      // note: three of these == operators are comparing std::optional
+      // against std::optional; as one would expect they are equal a)
+      // if both do not contain values or b) if both do and those
+      // contained values are the same
+      const bool conflict =
+       !(_user == rule_params.user &&
+         _acl_translation == rule_params.dest.acl_translation &&
+         _storage_class == rule_params.dest.storage_class &&
+         _mode == rule_params.mode);
+      if (conflict) {
+       *need_more_info = true;
+       return false;
+      }
     }
   }