On Sun, 17 Feb 2013, Loic Dachary wrote:
> When running
>
> bufferptr a("A", 1);
> bufferptr ab("AB", 2);
> a.cmp(ab);
>
> it returned zero because cmp only compared up to the length of the
> smallest buffer. The tests comparing the length of the buffers are
> moved before the memcmp comparing the actual content of the buffers.
>
> http://tracker.ceph.com/issues/4170 refs #4170
>
> Signed-off-by: Loic Dachary <loic@xxxxxxxxxxx>
The problem here is that for B cmp AB, we should be 1, because 'B' > 'A'.
The length only matters if we reach the end and everything so far is
equal.
> ---
> src/common/buffer.cc | 8 +-------
> src/test/bufferlist.cc | 14 ++++++++++++++
> 2 files changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/src/common/buffer.cc b/src/common/buffer.cc
> index e10d6c9..5a88849 100644
> --- a/src/common/buffer.cc
> +++ b/src/common/buffer.cc
> @@ -368,17 +368,11 @@ bool buffer_track_alloc = get_env_bool("CEPH_BUFFER_TRACK");
>
> int buffer::ptr::cmp(const ptr& o)
> {
> - int l = _len < o._len ? _len : o._len;
> - if (l) {
> - int r = memcmp(c_str(), o.c_str(), l);
> - if (!r)
> - return r;
I think this is the bug.. it should be if (r) return r, and fall through
below only if r == 0 because the buffers are equal.
sage
> - }
> if (_len < o._len)
> return -1;
> if (_len > o._len)
> return 1;
> - return 0;
> + return memcmp(c_str(), o.c_str(), _len);
> }
>
> bool buffer::ptr::is_zero() const
> diff --git a/src/test/bufferlist.cc b/src/test/bufferlist.cc
> index 71c2e79..aac41c6 100644
> --- a/src/test/bufferlist.cc
> +++ b/src/test/bufferlist.cc
> @@ -9,6 +9,20 @@
>
> #define MAX_TEST 1000000
>
> +TEST(BufferPtr, cmp) {
> + bufferptr empty;
> + bufferptr a("A", 1);
> + bufferptr ab("AB", 2);
> + bufferptr af("AF", 2);
> + EXPECT_GE(-1, empty.cmp(a));
> + EXPECT_LE(1, a.cmp(empty));
> + EXPECT_GE(-1, a.cmp(ab));
> + EXPECT_LE(1, ab.cmp(a));
> + EXPECT_EQ(0, ab.cmp(ab));
> + EXPECT_GE(-1, ab.cmp(af));
> + EXPECT_LE(1, af.cmp(ab));
> +}
> +
> TEST(BufferList, zero) {
> //
> // void zero()
> --
> 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
