From: Josh Durgin <josh.durgin@inktank.com>
Date: Fri, 21 Feb 2014 23:52:59 +0000 (-0800)
Subject: librados: don't destroy ObjListCtx when iterator reaches the end
X-Git-Tag: v0.78~129^2~1
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=8a0017ac2b982be3b83940b44c82e705898fa1d6;p=ceph.git

librados: don't destroy ObjListCtx when iterator reaches the end

There's no need to destroy it when we've used it once. It'll get destroyed
by the destructor anyway. Now that there's a seek method, we might want seek
and iterate again anyway, which is not possible if the ObjListCtx is gone.

Change get_next() to check the underlying ObjListCtx for the end of iteration
instead of deleting ObjListCtx, and modify the equality operator to count
the end marker (with ctx == NULL) as equal to having a ObjListCtx at the
end of its list.

Signed-off-by: Josh Durgin <josh.durgin@inktank.com>
---

diff --git a/src/librados/librados.cc b/src/librados/librados.cc
index ed4fb86f6ab3..5950a93a64ab 100644
--- a/src/librados/librados.cc
+++ b/src/librados/librados.cc
@@ -481,11 +481,15 @@ librados::ObjectIterator::~ObjectIterator()
 }
 
 bool librados::ObjectIterator::operator==(const librados::ObjectIterator& rhs) const {
-  return (ctx.get() == rhs.ctx.get());
+  if (ctx.get() == NULL)
+    return rhs.ctx.get() == NULL || rhs.ctx->lc->at_end();
+  if (rhs.ctx.get() == NULL)
+    return ctx.get() == NULL || ctx->lc->at_end();
+  return ctx.get() == rhs.ctx.get();
 }
 
 bool librados::ObjectIterator::operator!=(const librados::ObjectIterator& rhs) const {
-  return (ctx.get() != rhs.ctx.get());
+  return !(*this == rhs);
 }
 
 const pair<std::string, std::string>& librados::ObjectIterator::operator*() const {
@@ -519,10 +523,10 @@ uint32_t librados::ObjectIterator::seek(uint32_t pos)
 void librados::ObjectIterator::get_next()
 {
   const char *entry, *key;
+  if (ctx->lc->at_end())
+    return;
   int ret = rados_objects_list_next(ctx.get(), &entry, &key);
   if (ret == -ENOENT) {
-    ctx.reset();
-    *this = __EndObjectIterator;
     return;
   }
   else if (ret) {
diff --git a/src/test/librados/list.cc b/src/test/librados/list.cc
index 0d195bfed540..1c8e4df947b9 100644
--- a/src/test/librados/list.cc
+++ b/src/test/librados/list.cc
@@ -47,6 +47,32 @@ TEST_F(LibRadosListPP, ListObjectsPP) {
   ASSERT_TRUE(foundit);
 }
 
+TEST_F(LibRadosListPP, ListObjectsTwicePP) {
+  char buf[128];
+  memset(buf, 0xcc, sizeof(buf));
+  bufferlist bl1;
+  bl1.append(buf, sizeof(buf));
+  ASSERT_EQ((int)sizeof(buf), ioctx.write("foo", bl1, sizeof(buf), 0));
+  ObjectIterator iter(ioctx.objects_begin());
+  bool foundit = false;
+  while (iter != ioctx.objects_end()) {
+    foundit = true;
+    ASSERT_EQ((*iter).first, "foo");
+    ++iter;
+  }
+  ASSERT_TRUE(foundit);
+  ++iter;
+  ASSERT_TRUE(iter == ioctx.objects_end());
+  foundit = false;
+  iter.seek(0);
+  while (iter != ioctx.objects_end()) {
+    foundit = true;
+    ASSERT_EQ((*iter).first, "foo");
+    ++iter;
+  }
+  ASSERT_TRUE(foundit);
+}
+
 static void check_list(std::set<std::string>& myset, rados_list_ctx_t& ctx)
 {
   const char *entry;