fix operator>=(bufferlist& l, bufferlist& r)

  bufferlist a;
  a.append("A");
  bufferlist ab;
  ab.append("AB");

a >= ab failed, throwing an instance of 'ceph::buffer::end_of_buffer'
because it tried to access a[1]. All comparison operators should be
tested using a lexicographic sort like strcmp or memcmp (-1, 0, 1).
In the meantime, the missing test is added:

  if (l.length() == p && r.length() > p) return false;

A set of unit tests demonstrating the problem and covering all comparison
operators are added to show that the proposed fix works as expected.

http://tracker.ceph.com/issues/4157 refs #4157

Signed-off-by: Loic Dachary <loic@dachary.org>

diff --git a/src/include/buffer.h b/src/include/buffer.h
index 4f87ed7453b8637fb7d360a4b0d8f662466af4e1..b84e7f4746a54a0f0935d91083eca7820ce1743c 100644 (file)
--- a/src/include/buffer.h
+++ b/src/include/buffer.h
@@ -467,6 +467,7 @@ inline bool operator>=(bufferlist& l, bufferlist& r) {
   for (unsigned p = 0; ; p++) {
     if (l.length() > p && r.length() == p) return true;
     if (r.length() == p && l.length() == p) return true;
+    if (l.length() == p && r.length() > p) return false;
     if (l[p] > r[p]) return true;
     if (l[p] < r[p]) return false;
   }

diff --git a/src/test/bufferlist.cc b/src/test/bufferlist.cc
index 91e37e6b9f5b3d054101fdae6812e68060a34b6f..71c2e793c8260dd08b5410e28892a64bd9781ca1 100644 (file)
--- a/src/test/bufferlist.cc
+++ b/src/test/bufferlist.cc
@@ -57,6 +57,53 @@ TEST(BufferList, zero) {
   }
 }
 
+TEST(BufferList, compare) {
+  bufferlist a;
+  a.append("A");
+  bufferlist ab;
+  ab.append("AB");
+  bufferlist ac;
+  ac.append("AC");
+  //
+  // bool operator>(bufferlist& l, bufferlist& r)
+  //
+  ASSERT_FALSE(a > ab);
+  ASSERT_TRUE(ab > a);
+  ASSERT_TRUE(ac > ab);
+  ASSERT_FALSE(ab > ac);
+  ASSERT_FALSE(ab > ab);
+  //
+  // bool operator>=(bufferlist& l, bufferlist& r)
+  //
+  ASSERT_FALSE(a >= ab);
+  ASSERT_TRUE(ab >= a);
+  ASSERT_TRUE(ac >= ab);
+  ASSERT_FALSE(ab >= ac);
+  ASSERT_TRUE(ab >= ab);
+  //
+  // bool operator<(bufferlist& l, bufferlist& r)
+  //
+  ASSERT_TRUE(a < ab);
+  ASSERT_FALSE(ab < a);
+  ASSERT_FALSE(ac < ab);
+  ASSERT_TRUE(ab < ac);
+  ASSERT_FALSE(ab < ab);
+  //
+  // bool operator<=(bufferlist& l, bufferlist& r)
+  //
+  ASSERT_TRUE(a <= ab);
+  ASSERT_FALSE(ab <= a);
+  ASSERT_FALSE(ac <= ab);
+  ASSERT_TRUE(ab <= ac);
+  ASSERT_TRUE(ab <= ab);
+  //
+  // bool operator==(bufferlist &l, bufferlist &r)
+  //
+  ASSERT_FALSE(a == ab);
+  ASSERT_FALSE(ac == ab);
+  ASSERT_TRUE(ab == ab);
+}
+
 TEST(BufferList, EmptyAppend) {
   bufferlist bl;
   bufferptr ptr;