On 25 February 2013 19:52, Caroline Tice wrote: > I got too excited about being done and forgot to attach the patch. :-( > Sorry. Here it is.
Some comments follow, mostly from reading the comments to understand what this patch does, it's a really interesting feature! The generated files (configure, */Makefile.in) don't need to be in the patch and their ChangeLog entry can be simply "Regenerated" Was configure regenerated of modified by hand? When regenerating it with Autoconf 2.64 I get a different output. I also get this warning when regenerating the configury bits: src/Makefile.am:80: variable `libvtv___la_LIBADD' is defined but no program or src/Makefile.am:80: library has `libvtv___la' as canonical name (possible typo) The copyright dates should be updated to 2012-2013. vtv_add_to_log in libstdc++-v3/libsupc++/vtv_utils.cc uses va_start but the matching call to va_end is missing. Am I right in thinking there's no danger of namespace pollution from vtv_map.h etc. because those headers will never be included unless explicitly requested by users? I'm not sure what the rules are regarding flexible array members in C++ (as it's an extension) but it looks like insert_only_hash_map is a non-POD (formally, it has non-trivial initialization) but no constructor or destructor runs for it, so formally the object's lifetime never begins or ends, it is just a block of memory that gets allocated, some bytes are set, then the memory is deallocated again. The comments in libstdc++-v3/libsupc++/vtv_init.cc have a typo: +/* This file contains all the definitions that go into the libvtv_init + library, which is part of the vtable verification feature. This + library should contain exactly two functionsa (__VLTunprotect and s/functionsa/functions/ And in the comments in libstdc++-v3/libsupc++/vtv_rts.cc + The actual set of valid vtable pointers for a virtual class, Should that be "polymorphic class" instead of "virtual class"? Further on there are a few typos: + pointters for the class, so we wrote our own hashtable-based symbol s/pointters/pointers/ + libvtv_init.so is built from vtv_init.cc. It is designed to hel[p s/hel\[p/help/ + __VLTVerifyVtablePoitner) with stub functions that do nothing. If s/Poitner/Pointer/ + initialize any of these statics with a runtime call (for ex: + sysconf. (Unclosed parenthesis) + the secttion offset and size, in conjunction with the data in INFO s/secttion/section/ + /* TODO: Meed to revisit this code for dlopen. It most probably + is not unlocking the protected vtable vars after for a load s/Meed/Need/ s/after for a load/after a load/ I see a few TODO comments in the code, I assume the plan is to address them eventually as time permits, rather than this being a code-drop that becomes abandonware :-)
