Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Remove "as_error" and "error" from buffer packer.

Overrunning a buffer is a fatal error, and the typical usage pattern was
something like:

    <do packing>
    assert(!pa.error());

If there was a spot that did not assert this, it was considered a bug.  Rolling
this into the packer directly avoids bugs and is easier on the eyes.
  • Loading branch information...
commit 43e352804adaa6cd4e802b93cf84ee965eda27f5 1 parent 6f03553
@rescrv authored
Showing with 42 additions and 101 deletions.
  1. +40 −96 buffer.cc
  2. +2 −5 e/buffer.h
View
136 buffer.cc
@@ -28,7 +28,6 @@
#define __STDC_LIMIT_MACROS
// C
-#include <cassert>
#include <cstddef>
#include <cstring>
@@ -41,6 +40,7 @@
#include <iostream>
// e
+#include "e/assert.h"
#include "e/buffer.h"
#include "e/endian.h"
@@ -107,7 +107,7 @@ e :: buffer :: pack_at(uint32_t off)
void
e :: buffer :: resize(uint32_t sz) throw ()
{
- assert(sz <= m_cap);
+ EASSERT(sz <= m_cap);
m_size = sz;
}
@@ -161,40 +161,24 @@ e :: buffer :: buffer(const char* buf, uint32_t sz)
e :: buffer :: packer :: packer(buffer* buf, uint32_t off)
: m_buf(buf)
, m_off(off)
- , m_error(off > m_buf->m_cap)
{
+ EASSERT(off <= m_buf->m_cap);
}
e :: buffer :: packer :: packer(const packer& p)
: m_buf(p.m_buf)
, m_off(p.m_off)
- , m_error(p.m_error)
{
}
e::buffer::packer
-e :: buffer :: packer :: as_error() const
-{
- packer ret(*this);
- ret.m_error = true;
- return ret;
-}
-
-e::buffer::packer
e :: buffer :: packer :: copy(const slice& from)
{
uint64_t newsize = m_off + from.size();
-
- if (!m_error && newsize <= m_buf->m_cap)
- {
- memmove(m_buf->m_data + m_off, from.data(), from.size());
- m_buf->m_size = std::max(m_buf->m_size, static_cast<uint32_t>(newsize));
- return packer(m_buf, newsize);
- }
- else
- {
- return as_error();
- }
+ EASSERT(newsize <= m_buf->m_cap);
+ memmove(m_buf->m_data + m_off, from.data(), from.size());
+ m_buf->m_size = std::max(m_buf->m_size, static_cast<uint32_t>(newsize));
+ return packer(m_buf, newsize);
}
e::buffer::packer
@@ -229,112 +213,72 @@ e::buffer::packer
e :: buffer :: packer :: operator << (uint8_t rhs)
{
uint64_t newsize = m_off + sizeof(uint8_t);
-
- if (!m_error && newsize <= m_buf->m_cap)
- {
- e::pack8be(rhs, m_buf->m_data + m_off);
- m_buf->m_size = std::max(m_buf->m_size, static_cast<uint32_t>(newsize));
- return packer(m_buf, newsize);
- }
- else
- {
- return as_error();
- }
+ EASSERT(newsize <= m_buf->m_cap);
+ e::pack8be(rhs, m_buf->m_data + m_off);
+ m_buf->m_size = std::max(m_buf->m_size, static_cast<uint32_t>(newsize));
+ return packer(m_buf, newsize);
}
e::buffer::packer
e :: buffer :: packer :: operator << (uint16_t rhs)
{
uint64_t newsize = m_off + sizeof(uint16_t);
-
- if (!m_error && newsize <= m_buf->m_cap)
- {
- e::pack16be(rhs, m_buf->m_data + m_off);
- m_buf->m_size = std::max(m_buf->m_size, static_cast<uint32_t>(newsize));
- return packer(m_buf, newsize);
- }
- else
- {
- return as_error();
- }
+ EASSERT(newsize <= m_buf->m_cap);
+ e::pack16be(rhs, m_buf->m_data + m_off);
+ m_buf->m_size = std::max(m_buf->m_size, static_cast<uint32_t>(newsize));
+ return packer(m_buf, newsize);
}
e::buffer::packer
e :: buffer :: packer :: operator << (uint32_t rhs)
{
uint64_t newsize = m_off + sizeof(uint32_t);
-
- if (!m_error && newsize <= m_buf->m_cap)
- {
- e::pack32be(rhs, m_buf->m_data + m_off);
- m_buf->m_size = std::max(m_buf->m_size, static_cast<uint32_t>(newsize));
- return packer(m_buf, newsize);
- }
- else
- {
- return as_error();
- }
+ EASSERT(newsize <= m_buf->m_cap);
+ e::pack32be(rhs, m_buf->m_data + m_off);
+ m_buf->m_size = std::max(m_buf->m_size, static_cast<uint32_t>(newsize));
+ return packer(m_buf, newsize);
}
e::buffer::packer
e :: buffer :: packer :: operator << (uint64_t rhs)
{
uint64_t newsize = m_off + sizeof(uint64_t);
-
- if (!m_error && newsize <= m_buf->m_cap)
- {
- e::pack64be(rhs, m_buf->m_data + m_off);
- m_buf->m_size = std::max(m_buf->m_size, static_cast<uint32_t>(newsize));
- return packer(m_buf, newsize);
- }
- else
- {
- return as_error();
- }
+ EASSERT(newsize <= m_buf->m_cap);
+ e::pack64be(rhs, m_buf->m_data + m_off);
+ m_buf->m_size = std::max(m_buf->m_size, static_cast<uint32_t>(newsize));
+ return packer(m_buf, newsize);
}
e::buffer::packer
e :: buffer :: packer :: operator << (const slice& rhs)
{
uint64_t newsize = m_off + sizeof(uint32_t) + rhs.size();
-
- if (!m_error && newsize <= m_buf->m_cap && rhs.size() <= UINT32_MAX)
- {
- uint32_t sz = rhs.size();
- *this << sz; // XXX perf loss
- memmove(m_buf->m_data + m_off + sizeof(uint32_t), rhs.data(), sz);
- m_buf->m_size = std::max(m_buf->m_size, static_cast<uint32_t>(newsize));
- return packer(m_buf, newsize);
- }
- else
- {
- return as_error();
- }
+ EASSERT(newsize <= m_buf->m_cap);
+ EASSERT(rhs.size() <= UINT32_MAX);
+ uint32_t sz = rhs.size();
+ e::pack32be(sz, m_buf->m_data + m_off);
+ memmove(m_buf->m_data + m_off + sizeof(uint32_t), rhs.data(), sz);
+ m_buf->m_size = std::max(m_buf->m_size, static_cast<uint32_t>(newsize));
+ return packer(m_buf, newsize);
}
e::buffer::packer
e :: buffer :: packer :: operator << (const buffer::padding& rhs)
{
uint64_t newsize = m_off + rhs.m_pad;
+ EASSERT(newsize <= m_buf->m_cap);
- if (!m_error && newsize <= m_buf->m_cap)
- {
- // Zero the new bytes which the padding adds to the buffer's size.
- // The padded region is not zeroed absolutely because the padding can be
- // used to skip regions (e.g., headers) so that the packer can pack
- // after the header.
- if (m_buf->m_size < newsize)
- {
- memset(m_buf->m_data + m_buf->m_size, 0, newsize - m_buf->m_size);
- m_buf->m_size = newsize;
- }
-
- return packer(m_buf, newsize);
- }
- else
+ // Zero the new bytes which the padding adds to the buffer's size.
+ // The padded region is not zeroed absolutely because the padding can be
+ // used to skip regions (e.g., headers) so that the packer can pack
+ // after the header.
+ if (m_buf->m_size < newsize)
{
- return as_error();
+ memset(m_buf->m_data + m_buf->m_size, 0, newsize - m_buf->m_size);
+ m_buf->m_size = newsize;
}
+
+ return packer(m_buf, newsize);
}
e :: buffer :: unpacker :: unpacker(const buffer* buf, uint32_t off)
View
7 e/buffer.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011, Robert Escriva
+// Copyright (c) 2011-2012, Robert Escriva
// All rights reserved.
//
// Redistribution and use in source and binary forms, with or without
@@ -106,8 +106,6 @@ class buffer::packer
packer(const packer& p);
public:
- packer as_error() const;
- bool error() const { return m_error; }
uint32_t remain() const { return m_buf->m_cap - m_off; }
// Unlike the operator on slices, this does not pack the size. It
// simply copies the contents of "from" and advances the internal
@@ -131,7 +129,6 @@ class buffer::packer
private:
buffer* m_buf;
uint32_t m_off;
- bool m_error;
};
class buffer::padding
@@ -208,7 +205,7 @@ e :: buffer :: packer :: operator << (const std::vector<T>& rhs)
uint32_t sz = rhs.size();
e::buffer::packer ret = *this << sz;
- for (uint32_t i = 0; !ret.error() && i < sz; ++i)
+ for (uint32_t i = 0; i < sz; ++i)
{
ret = ret << rhs[i];
}
Please sign in to comment.
Something went wrong with that request. Please try again.