RGW: make SSTR macro safe against variable name collisions

The original SSTR macro defined a local variable `std::stringstream ss`
inside its expansion. When an expression that used a variable named `ss`
is passed to this macro, it led to name shadowing and unexpected results.

  Example:
    std::stringstream ss;
    ss << "aaa";
    auto result = SSTR("this is ss=" << ss.str());

  Actual result:
    "this is ss=this is ss="
  Expected result:
    "this is ss=aaa"

This change rewrites the macro to construct an unnamed temporary to
avoid any name collision.

Signed-off-by: Oguzhan Ozmen <oozmen@bloomberg.net>

diff --git a/src/rgw/driver/rados/rgw_sync_trace.h b/src/rgw/driver/rados/rgw_sync_trace.h
index 9686d19271ef122ca5613c07094382d00c67f233..074844ad5010d61eae04bede1e5fa5bb8d95bfcd 100644 (file)
--- a/src/rgw/driver/rados/rgw_sync_trace.h
+++ b/src/rgw/driver/rados/rgw_sync_trace.h
@@ -15,11 +15,7 @@
 #include <shared_mutex>
 #include <boost/circular_buffer.hpp>
 
-#define SSTR(o) ({      \
-  std::stringstream ss; \
-  ss << o;              \
-  ss.str();             \
-})
+#define SSTR(o) ((std::ostringstream{} << o).str())
 
 #define RGW_SNS_FLAG_ACTIVE   1
 #define RGW_SNS_FLAG_ERROR    2

diff --git a/src/test/rgw/CMakeLists.txt b/src/test/rgw/CMakeLists.txt
index eabbc73fc0a03a285a02335baacfca0c7558250b..4fd8d390483cc6807e648ad45f62a62308ae7561 100644 (file)
--- a/src/test/rgw/CMakeLists.txt
+++ b/src/test/rgw/CMakeLists.txt
@@ -316,6 +316,13 @@ target_include_directories(unittest_rgw_url
   SYSTEM PRIVATE "${CMAKE_SOURCE_DIR}/src/rgw")
 target_link_libraries(unittest_rgw_url ${rgw_libs})
 
+# unittest_rgw_sync_trace
+add_executable(unittest_rgw_sync_trace test_rgw_sync_trace.cc)
+add_ceph_unittest(unittest_rgw_sync_trace)
+target_include_directories(unittest_rgw_sync_trace
+  SYSTEM PRIVATE "${CMAKE_SOURCE_DIR}/src/rgw")
+target_link_libraries(unittest_rgw_sync_trace)
+
 if(WITH_RADOSGW_RADOS)
 add_executable(ceph_test_rgw_gc_log test_rgw_gc_log.cc $<TARGET_OBJECTS:unit-main>)
 target_include_directories(ceph_test_rgw_gc_log

diff --git a/src/test/rgw/test_rgw_sync_trace.cc b/src/test/rgw/test_rgw_sync_trace.cc
new file mode 100644 (file)
index 0000000..d0bcca0
--- /dev/null
+++ b/src/test/rgw/test_rgw_sync_trace.cc
@@ -0,0 +1,23 @@
+// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:nil -*-
+// vim: ts=8 sw=2 sts=2 expandtab ft=cpp
+
+#include <gtest/gtest.h>
+#include "rgw/driver/rados/rgw_sync_trace.h"
+
+TEST(TestSSTR, using_a_var_named_ss)
+{
+  std::stringstream ss;
+
+  ss << "aaa";
+  auto result = SSTR("this is ss=" << ss.str());
+  ASSERT_EQ(result, "this is ss=aaa");
+}
+
+TEST(TestSSTR, using_a_var_named_other_than_ss)
+{
+  std::stringstream ss2;
+
+  ss2 << "aaa";
+  auto result = SSTR("this is ss2=" << ss2.str());
+  ASSERT_EQ(result, "this is ss2=aaa");
+}