[MOAB-dev] r3431 - MOAB/trunk

[Tim Tautges]

Whew, thanks.  There goes my thought of how to implement unit tests.  I though other codes did this sort of thing too - 
what do you do about unit tests in msq?

- tim

kraftche at cae.wisc.edu wrote:
> Author: kraftche
> Date: 2010-01-18 11:52:33 -0600 (Mon, 18 Jan 2010)
> New Revision: 3431
> 
> Modified:
>    MOAB/trunk/Tqdcfr.cpp
>    MOAB/trunk/Tqdcfr.hpp
> Log:
> Fix memory corruption in tqdcfr test (two redundant fixes for the same problem).
> 
> The problem:
> 
> Tqdcfr.cpp contains both the implementation of MOAB's cub file reader and 
> some test code.  The test code is wrapped within a #ifdef TEST_TQDCFR 
> block.  When the test is compiled and linked to libmoab.so the resulting
> executable ends up with two copies of the Tqdcfr implementation: one from
> MOAB and one from the test .o file.  When libmoab is a static library this
> isn't a problem.  The linker will never pull in MOAB's Tqdcfr.o from the
> static library because it doesn't require it to resolve any symbols (all
> symbols already defined in the test .o.)  This is not the case for a shared
> library, where the .so is either used entirely or not at all.  The duplicate
> code isn't really an issue in that case either as long as both copies are
> the same.  However, Tqdcfr also declares some static instances of std::string.
> That's where things go wrong.  The resulting test executable ends up with
> two instances of the static string with the same symbol name and two registered
> exit handlers to clean up the statics (one form the .so and one from the
> main.)  These both end up trying to free the same instance of the static
> string, resulting in memory corruption.
> 
> The 1st implemented solution:  Use const char* defined in .cpp file rather
> than std::string for string constants so no exit-time cleanup is required.
> 
> The 2nd implemented solution:  Wrap main body of Tqdcfr.cpp in 
> #ifndef TEST_TQDCFR to avoid object code duplication.
> 
> The best solution (not implemented) is probably to just put the test code in 
> its own .cpp file.
> 
> 
> 
> Modified: MOAB/trunk/Tqdcfr.cpp
> ===================================================================
> --- MOAB/trunk/Tqdcfr.cpp	2010-01-16 00:07:47 UTC (rev 3430)
> +++ MOAB/trunk/Tqdcfr.cpp	2010-01-18 17:52:33 UTC (rev 3431)
> @@ -16,12 +16,16 @@
>  #include "Tqdcfr.hpp"
>  #include "MBCore.hpp"
>  #include "MBRange.hpp"
> +#include "FileOptions.hpp"
> +#include <iostream>
> +
> +#ifndef TEST_TQDCFR
> +
>  #include "MBReadUtilIface.hpp"
>  #include "GeomTopoTool.hpp"
>  #include "MBTagConventions.hpp"
>  #include "MBCN.hpp"
>  #include "MBInternals.hpp"
> -#include "FileOptions.hpp"
>  #include "HigherOrderFactory.hpp"
>  #include "exodus_order.h"
>  
> @@ -29,7 +33,6 @@
>  #include "MBmpi.h"
>  #endif
>  
> -#include <iostream>
>  #include <sstream>
>  #include <assert.h>
>  
> @@ -91,8 +94,8 @@
>  // moab_conn[ cub_hex27_order[i] ] = cubit_conn[ i ];
>  const int* const* const* const cub_elem_order_map = exodus_elem_order_map;
>  
> -std::string Tqdcfr::BLOCK_NODESET_OFFSET_TAG_NAME = "BLOCK_NODESET_OFFSET";
> -std::string Tqdcfr::BLOCK_SIDESET_OFFSET_TAG_NAME = "BLOCK_SIDESET_OFFSET";
> +const char *const BLOCK_NODESET_OFFSET_TAG_NAME = "BLOCK_NODESET_OFFSET";
> +const char *const BLOCK_SIDESET_OFFSET_TAG_NAME = "BLOCK_SIDESET_OFFSET";
>  
>  #define RR if (MB_SUCCESS != result) return result
>  
> @@ -400,7 +403,7 @@
>      // set, we don't need to convert
>    unsigned int nodeset_offset, sideset_offset;
>    MBTag tmp_tag;
> -  MBErrorCode result = mdbImpl->tag_get_handle(BLOCK_NODESET_OFFSET_TAG_NAME.c_str(),
> +  MBErrorCode result = mdbImpl->tag_get_handle(BLOCK_NODESET_OFFSET_TAG_NAME,
>                                                 tmp_tag);
>    if (MB_SUCCESS != result) nodeset_offset = 0;
>    else {
> @@ -408,7 +411,7 @@
>      if (MB_SUCCESS != result) return result;
>    }
>  
> -  result = mdbImpl->tag_get_handle(BLOCK_SIDESET_OFFSET_TAG_NAME.c_str(),
> +  result = mdbImpl->tag_get_handle(BLOCK_SIDESET_OFFSET_TAG_NAME,
>                                     tmp_tag);
>    if (MB_SUCCESS != result) sideset_offset = 0;
>    else {
> @@ -2571,7 +2574,8 @@
>    return mdbImpl->create_meshset( flags, h );
>  }
>  
> -#ifdef TEST_TQDCFR
> +// #ifdef TEST_TQDCFR
> +#else
>  #include "MBCore.hpp"
>  #include "testdir.h"
>  
> 
> Modified: MOAB/trunk/Tqdcfr.hpp
> ===================================================================
> --- MOAB/trunk/Tqdcfr.hpp	2010-01-16 00:07:47 UTC (rev 3430)
> +++ MOAB/trunk/Tqdcfr.hpp	2010-01-18 17:52:33 UTC (rev 3431)
> @@ -336,9 +336,6 @@
>  
>    Tqdcfr(MBInterface *impl);
>  
> -  static std::string BLOCK_NODESET_OFFSET_TAG_NAME;
> -  static std::string BLOCK_SIDESET_OFFSET_TAG_NAME;
> -  
>    MBErrorCode create_set( MBEntityHandle& h, unsigned int flags = MESHSET_SET );
>  
>  private:
> 
> 

-- 
================================================================
"You will keep in perfect peace him whose mind is
   steadfast, because he trusts in you."               Isaiah 26:3

              Tim Tautges            Argonne National Laboratory
          (tautges at mcs.anl.gov)      (telecommuting from UW-Madison)
          phone: (608) 263-8485      1500 Engineering Dr.
            fax: (608) 263-4499      Madison, WI 53706