AK: Prevent confusing silent misuse of ByteBuffer

Thankfully, this hasn't happened in any other code yet, but it happened
while I was trying something out. Using '==' on two ByteBuffers to check
whether they're equal seemed straight-forward, so I ran into the trap.
This commit is contained in:
Ben Wiederhake 2020-08-22 16:10:19 +02:00 committed by Andreas Kling
parent 901ed9b85d
commit 4acdb60ba3
3 changed files with 74 additions and 10 deletions

52
AK/ByteBuffer.cpp Normal file
View file

@ -0,0 +1,52 @@
/*
* Copyright (c) 2020, the SerenityOS developers.
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
*
* 1. Redistributions of source code must retain the above copyright notice, this
* list of conditions and the following disclaimer.
*
* 2. Redistributions in binary form must reproduce the above copyright notice,
* this list of conditions and the following disclaimer in the documentation
* and/or other materials provided with the distribution.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
* DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
* FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
* DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
* SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
* CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
* OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
#include <AK/ByteBuffer.h>
namespace AK {
bool ByteBuffer::operator==(const ByteBuffer& other) const
{
if (is_empty() != other.is_empty())
return false;
if (is_empty())
return true;
if (size() != other.size())
return false;
// So they both have data, and the same length.
// Avoid hitting conditionals in ever iteration.
size_t n = size();
const u8* this_data = data();
const u8* other_data = other.data();
for (size_t i = 0; i < n; ++i) {
if (this_data[i] != other_data[i])
return false;
}
return true;
}
}

View file

@ -147,6 +147,14 @@ public:
bool operator!() const { return is_null(); }
bool is_null() const { return m_impl == nullptr; }
// Disable default implementations that would use surprising integer promotion.
bool operator==(const ByteBuffer& other) const;
bool operator!=(const ByteBuffer& other) const { return !(*this == other); }
bool operator<=(const ByteBuffer& other) const = delete;
bool operator>=(const ByteBuffer& other) const = delete;
bool operator<(const ByteBuffer& other) const = delete;
bool operator>(const ByteBuffer& other) const = delete;
u8& operator[](size_t i)
{
ASSERT(m_impl);

View file

@ -53,17 +53,21 @@ TEST_CASE(equality_operator)
EXPECT_EQ(d == d, true);
}
TEST_CASE(other_operators)
/*
* FIXME: These `negative_*` tests should cause precisely one compilation error
* each, and always for the specified reason. Currently we do not have a harness
* for that, so in order to run the test you need to set the #define to 1, compile
* it, and check the error messages manually.
*/
#define COMPILE_NEGATIVE_TESTS 0
#if COMPILE_NEGATIVE_TESTS
TEST_CASE(negative_operator_lt)
{
ByteBuffer a = ByteBuffer::copy("Hello, world", 7);
ByteBuffer b = ByteBuffer::copy("Hello, friend", 7);
// `a` and `b` are both "Hello, ".
ByteBuffer c = ByteBuffer::copy("asdf", 4);
ByteBuffer d;
EXPECT_EQ(a < a, true);
EXPECT_EQ(a <= b, true);
EXPECT_EQ(a >= c, false);
EXPECT_EQ(a > d, false);
ByteBuffer a = ByteBuffer::copy("Hello, world", 10);
ByteBuffer b = ByteBuffer::copy("Hello, friend", 10);
(void)(a < b);
// error: error: use of deleted function bool AK::ByteBuffer::operator<(const AK::ByteBuffer&) const
}
#endif /* COMPILE_NEGATIVE_TESTS */
TEST_MAIN(ByteBuffer)