Applied. Thanks, Loic!
On Sat, 16 Feb 2013, Loic Dachary wrote:
> 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@xxxxxxxxxxx>
> ---
> src/include/buffer.h | 1 +
> src/test/bufferlist.cc | 47 +++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 48 insertions(+)
>
> diff --git a/src/include/buffer.h b/src/include/buffer.h
> index 4f87ed7..b84e7f4 100644
> --- 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 91e37e6..71c2e79 100644
> --- 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;
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
