> -----Original Message-----
> From: Farid Zaripov [mailto:[EMAIL PROTECTED]
> Sent: Monday, July 17, 2006 6:54 PM
> To: [email protected]
> Subject: RE: svn commit: r421918 - in
> /incubator/stdcxx/trunk/tests: include/rw_alloc.h src/alloc.cpp
> > The error handling in the file isn't very robust: we shouldn't just
> > assert that the system calls succeeded; we should print out
> an error
> > message and either exit with a non-zero exit status or
> return a value
> > indicating a failure (if possible). If the former, the mechanism to
> > use is
> > rw_error() and/or rw_fatal(). The %m and %{#m} directives let you
> > format the text of the error message corresponding to the value of
> > errno and the name of the EXXX variable, respectively.
>
> Done. The diff file is attached.
I have accidentally sent the letter from Outlook and attachment have
been cut out.
Farid.
Index: alloc.cpp
===================================================================
--- alloc.cpp (revision 422655)
+++ alloc.cpp (working copy)
@@ -30,6 +30,7 @@
#include <string.h> // for memset()
#include <rw_alloc.h>
+#include <driver.h> // for rw_error(), rw_fatal()
#ifdef __CYGWIN__
@@ -205,7 +206,8 @@
{
for (size_t dist = last - first; dist > 0; ) {
- const size_t half = dist / 2;
+ // half = dist / 2
+ const size_t half = dist >> 1;
Pair* const middle = first + half;
if (middle->addr_ < addr) {
@@ -243,20 +245,25 @@
static const size_t pagemask = GETPAGESIZE () - 1;
// check that pagesize is power of 2
- RW_ASSERT (0 == ((pagemask + 1) & pagemask));
+ rw_fatal (0 == ((pagemask + 1) & pagemask),
+ __FILE__, __LINE__,
+ "expected pagesize is power of 2, got %zu",
+ pagemask + 1);
// addr_ should be aligned to memory page boundary
size_t off = size_t (addr) & pagemask;
addr_ = _RWSTD_STATIC_CAST(char*, addr) - off;
size_ = size + off;
int res = mprotect (addr_, size, PROT_READ | PROT_WRITE);
- RW_ASSERT (0 == res);
+ rw_error (0 == res, __FILE__, __LINE__,
+ "mprotect failed: errno = %{#m} (%{m})");
}
~MemRWGuard ()
{
int res = mprotect (addr_, size_, PROT_READ);
- RW_ASSERT (0 == res);
+ rw_error (0 == res, __FILE__, __LINE__,
+ "mprotect failed: errno = %{#m} (%{m})");
}
private:
@@ -274,7 +281,8 @@
return;
int res = munmap (table_, table_size_);
- RW_ASSERT (0 == res);
+ rw_error (0 == res, __FILE__, __LINE__,
+ "munmap failed: errno = %{#m} (%{m})");
table_ = 0;
table_size_ = 0;
@@ -286,7 +294,11 @@
_rw_table_grow ()
{
// table_max_size_ cannot be less than allocated blocks
- RW_ASSERT (table_max_size_ >= _rw_stats.blocks_);
+ rw_fatal (table_max_size_ >= _rw_stats.blocks_,
+ __FILE__, __LINE__,
+ "table_max_size_ = %zu; _rw_stats.blocks_ = %zu: "
+ "table_max_size_ shouldn't be less than _rw_stats.blocks_",
+ table_max_size_, _rw_stats.blocks_);
// check for free space in current table
if (table_max_size_ == _rw_stats.blocks_) {
@@ -306,7 +318,8 @@
// protect the new table
int res = mprotect (new_table, new_table_size, PROT_READ);
- RW_ASSERT (0 == res);
+ rw_error (0 == res, __FILE__, __LINE__,
+ "mprotect failed: errno = %{#m} (%{m})");
// free old table
_rw_table_free ();
@@ -349,8 +362,11 @@
static void
_rw_table_remove (Pair* it)
{
- RW_ASSERT (table_ <= it && table_ + _rw_stats.blocks_ > it);
- size_t index = it - table_;
+ int index = it - table_;
+ rw_fatal (0 <= index && int (_rw_stats.blocks_) > index,
+ __FILE__, __LINE__,
+ "invalid index in _rw_table_remove: %i",
+ index);
MemRWGuard guard (table_, table_size_);
memmove (it, it + 1, (--_rw_stats.blocks_ - index) * sizeof (Pair));
@@ -370,18 +386,21 @@
if (0 == blocks_per_page)
blocks_per_page = (pagesize - sizeof (Blocks)) / sizeof (BlockInfo) +
1;
- void* buf = mmap (0, pagesize, PROT_READ,
+ void* buf = mmap (0, pagesize, PROT_READ | PROT_WRITE,
MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
if (MAP_FAILED != buf) {
- MemRWGuard guard (buf, pagesize);
-
memset (buf, 0, pagesize);
Blocks* blocks = _RWSTD_STATIC_CAST(Blocks*, buf);
blocks->nblocks_ = blocks_per_page;
+ // set r/o access to the new page
+ int res = mprotect (buf, pagesize, PROT_READ);
+ rw_error (0 == res, __FILE__, __LINE__,
+ "mprotect failed: errno = %{#m} (%{m})");
+
if (0 == _rw_head)
_rw_head = blocks;
else
@@ -402,7 +421,9 @@
static void
_rw_free_blocks ()
{
- RW_ASSERT (0 == _rw_stats.blocks_);
+ rw_fatal (0 == _rw_stats.blocks_, __FILE__, __LINE__,
+ "_rw_free_blocks called when %zu blocks are not freed",
+ _rw_stats.blocks_);
static const size_t pagesize = GETPAGESIZE ();
@@ -410,7 +431,8 @@
Blocks* it = _rw_head;
_rw_head = _rw_head->next_;
int res = munmap (it, pagesize);
- RW_ASSERT (0 == res);
+ rw_error (0 == res, __FILE__, __LINE__,
+ "munmap failed: errno = %{#m} (%{m})");
}
_rw_tail = 0;
@@ -447,7 +469,9 @@
// res = _rw_find_unused_from (_rw_tail);
res = _rw_tail->blocks_;
// res should be != 0
- RW_ASSERT (0 != res);
+ rw_fatal (0 != res, __FILE__, __LINE__,
+ "logic error in _rw_find_unused: res == 0 after "
+ "_rw_allocate_blocks() succeeded ");
}
return res;
@@ -520,7 +544,10 @@
size_t size = nbytes + pagesize;
// check that pagesize is power of 2
- RW_ASSERT (0 == (pagesize & (pagesize - 1)));
+ rw_fatal (0 == (pagesize & (pagesize - 1)),
+ __FILE__, __LINE__,
+ "expected pagesize is power of 2, got %zu",
+ pagesize);
size_t offset = size & (pagesize - 1);
if (offset) {
@@ -543,7 +570,8 @@
// deny access to the guard page
int res = mprotect (guard, pagesize, PROT_NONE);
- RW_ASSERT (0 == res);
+ rw_error (0 == res, __FILE__, __LINE__,
+ "mprotect failed: errno = %{#m} (%{m})");
newinfo.size_ = size;
newinfo.data_ = data + offset;
@@ -579,7 +607,8 @@
free (addr);
else {
int res = munmap (info.addr_, info.size_);
- RW_ASSERT (0 == res);
+ rw_error (0 == res, __FILE__, __LINE__,
+ "munmap failed: errno = %{#m} (%{m})");
}
{
@@ -595,5 +624,8 @@
}
}
else
- RW_ASSERT (!"Invalid addr passed to the rw_free");
+ rw_assert (0 == addr, __FILE__, __LINE__,
+ "rw_free(%#p): the address is not a valid address, "
+ "returned by rw_alloc()",
+ addr);
}
