Commit 2b501f7f authored by Felix Handte's avatar Felix Handte Committed by Facebook Github Bot

IOBuf: Fix Bug in Cloning with Headroom and Tailroom

Summary:
If the source IOBuf is not chained, we short-circuit the cloning logic and
instead share the existing buffer. This works when we implicitly require the
clone to have the same headroom and tailroom requirements as the source
IOBuf. However, the API allows setting other values. We need to test that we
meet those requirements in order to use the shortcut. This diff adds those
checks.

Reviewed By: yfeldblum

Differential Revision: D18180574

fbshipit-source-id: 31bfaabd7663cf83faf919d8cb242df0a7a2969d
parent 802676ff
...@@ -611,7 +611,7 @@ IOBuf IOBuf::cloneCoalescedAsValue() const { ...@@ -611,7 +611,7 @@ IOBuf IOBuf::cloneCoalescedAsValue() const {
IOBuf IOBuf::cloneCoalescedAsValueWithHeadroomTailroom( IOBuf IOBuf::cloneCoalescedAsValueWithHeadroomTailroom(
std::size_t newHeadroom, std::size_t newHeadroom,
std::size_t newTailroom) const { std::size_t newTailroom) const {
if (!isChained()) { if (!isChained() && newHeadroom <= headroom() && newTailroom <= tailroom()) {
return cloneOneAsValue(); return cloneOneAsValue();
} }
// Coalesce into newBuf // Coalesce into newBuf
......
...@@ -1536,6 +1536,24 @@ TEST(IOBuf, CloneCoalescedChain) { ...@@ -1536,6 +1536,24 @@ TEST(IOBuf, CloneCoalescedChain) {
EXPECT_EQ(b->computeChainDataLength(), c.length()); // Same length EXPECT_EQ(b->computeChainDataLength(), c.length()); // Same length
gen.seed(fillSeed); gen.seed(fillSeed);
checkBuf(&c, gen); // Same contents checkBuf(&c, gen); // Same contents
auto newHeadroom = b->headroom() + 10;
auto newTailroom = b->tailroom();
c = b->cloneCoalescedAsValueWithHeadroomTailroom(newHeadroom, newTailroom);
EXPECT_FALSE(c.isChained()); // Not chained
EXPECT_GE(c.headroom(), newHeadroom);
EXPECT_GE(c.tailroom(), newTailroom);
gen.seed(fillSeed);
checkBuf(&c, gen); // Same contents
newHeadroom = b->headroom();
newTailroom = b->tailroom() + 10;
c = b->cloneCoalescedAsValueWithHeadroomTailroom(newHeadroom, newTailroom);
EXPECT_FALSE(c.isChained()); // Not chained
EXPECT_GE(c.headroom(), newHeadroom);
EXPECT_GE(c.tailroom(), newTailroom);
gen.seed(fillSeed);
checkBuf(&c, gen); // Same contents
} }
TEST(IOBuf, CloneCoalescedSingle) { TEST(IOBuf, CloneCoalescedSingle) {
...@@ -1553,6 +1571,24 @@ TEST(IOBuf, CloneCoalescedSingle) { ...@@ -1553,6 +1571,24 @@ TEST(IOBuf, CloneCoalescedSingle) {
EXPECT_EQ(b->capacity(), c->capacity()); EXPECT_EQ(b->capacity(), c->capacity());
EXPECT_EQ(b->data(), c->data()); EXPECT_EQ(b->data(), c->data());
EXPECT_EQ(b->length(), c->length()); EXPECT_EQ(b->length(), c->length());
auto newHeadroom = b->headroom() + 10;
auto newTailroom = b->tailroom();
c = b->cloneCoalescedWithHeadroomTailroom(newHeadroom, newTailroom);
EXPECT_FALSE(c->isChained()); // Not chained
EXPECT_EQ(
ByteRange(c->data(), c->length()), ByteRange(b->data(), b->length()));
EXPECT_GE(c->headroom(), newHeadroom);
EXPECT_GE(c->tailroom(), newTailroom);
newHeadroom = b->headroom();
newTailroom = b->tailroom() + 10;
c = b->cloneCoalescedWithHeadroomTailroom(newHeadroom, newTailroom);
EXPECT_FALSE(c->isChained()); // Not chained
EXPECT_EQ(
ByteRange(c->data(), c->length()), ByteRange(b->data(), b->length()));
EXPECT_GE(c->headroom(), newHeadroom);
EXPECT_GE(c->tailroom(), newTailroom);
} }
TEST(IOBuf, fillIov) { TEST(IOBuf, fillIov) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment