Re: svn commit: r421918 - in /incubator/stdcxx/trunk/tests: include/rw_alloc.h src/alloc.cpp

[Martin Sebor <se...@roguewave.com>]

antonp@apache.org wrote:
> Author: antonp
> Date: Fri Jul 14 07:54:34 2006
> New Revision: 421918
> 
> URL: http://svn.apache.org/viewvc?rev=421918&view=rev

A few more comments:

The (still unwritten) naming convention for names of symbols private
to the driver (i.e., symbols not to be used outside the driver,
regardless of their linkage) calls for an "_rw_" prefix (e.g.,
_rw_translate_prot). This applies to functions as well as variables
except for the Win32 POSIX compatibility layer, i.e., except for
functions like mmap() et al). I changed most of the names in my last
commit: http://svn.apache.org/viewvc?rev=422071&view=rev so it's
something to keep in mind in the future.

Instead of _rw_binary_search/_rw_lower_bound, would it be possible
to use the standard C function bsearch()?

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.

Finally, I'm not sure I understand the purpose of the MemRWGuard
class. The ctor sets one kind of permissions on a page of memory
and the dtor sets another. Some functions do things in between
the invocations of the two, others don't. It's not clear to me
whether all invocations of the ctor and dtor really need the dtor
to run or not. In other words, it's not clear to me that plain
old functions wouldn't be more appropriate. Can you shed some
light on this?

Thanks
Martin