Skip to content
  • Salamandar's avatar
    Fix test (dentry->d_name is invalidated by closedir...) (!41) · cdba5cee
    Salamandar authored and Mike Fleetwood's avatar Mike Fleetwood committed
    We have to copy the dentry->d_name before calling closedir().  If not,
    the string points to nothing and the test fails (It does not fail all
    the time, but only by chance).
    
    Confirmed using valgrind.  Selected output from running the unit test
    under valgrind:
    
      $ valgrind --track-origins=yes ./test_blockSpecial
      ==25110== Memcheck, a memory error detector
      ...
      ==25110== Command: ./test_BlockSpecial
      ==25110==
      Running main() from src/gtest_main.cc
      [==========] Running 26 tests from 1 test case.
      [----------] Global test environment set-up.
      [----------] 26 tests from BlockSpecialTest
      ...
      [ RUN      ] BlockSpecialTest.NamedBlockSpecialObjectBySymlinkMatches
      ==25110== Invalid read of size 1
      ==25110==    at 0x4C2C9B2: strlen (vg_replace_strmem.c:458)
      ==25110==    by 0x40E7C4: length (char_traits.h:259)
      ==25110==    by 0x40E7C4: append (basic_string.h:1009)
      ==25110==    by 0x40E7C4: operator+<char, std::char_traits<char>, std::allocator<char> > (basic_string.h:2468)
      ==25110==    by 0x40E7C4: get_link_name (test_BlockSpecial.cc:156)
      ==25110==    by 0x40E7C4: GParted::BlockSpecialTest_NamedBlockSpecialObjectBySymlinkMatches_Test::TestBody() (test_BlockSpecial.cc:247)
      ...
      =25110==  Address 0x1231ea93 is 115 bytes inside a block of size 32,816 free'd
      ==25110==    at 0x4C2ACBD: free (vg_replace_malloc.c:530)
      ==25110==    by 0x9F773AC: closedir (in /usr/lib64/libc-2.17.so)
      ==25110==    by 0x40E7AC: get_link_name (test_BlockSpecial.cc:153)
      ==25110==    by 0x40E7AC: GParted::BlockSpecialTest_NamedBlockSpecialObjectBySymlinkMatches_Test::TestBody() (test_BlockSpecial.cc:247)
      ...
      ==25110==  Block was alloc'd at
      ==25110==    at 0x4C29BC3: malloc (vg_replace_malloc.c:299)
      ==25110==    by 0x9F77280: __alloc_dir (in /usr/lib64/libc-2.17.so)
      ==25110==    by 0x40E746: get_link_name (test_BlockSpecial.cc:134)
      ==25110==    by 0x40E746: GParted::BlockSpecialTest_NamedBlockSpecialObjectBySymlinkMatches_Test::TestBody() (test_BlockSpecial.cc:247)
      ...
      ==25110== Invalid read of size 1
      ==25110==    at 0x4C2E220: memcpy@@GLIBC_2.14 (vg_replace_strmem.c:1022)
      ==25110==    by 0x953A997: std::string::append(char const*, unsigned long) (in /usr/lib64/libstdc++.so.6.0.19)
      ==25110==    by 0x40E7D2: append (basic_string.h:1009)
      ==25110==    by 0x40E7D2: operator+<char, std::char_traits<char>, std::allocator<char> > (basic_string.h:2468)
      ==25110==    by 0x40E7D2: get_link_name (test_BlockSpecial.cc:156)
      ==25110==    by 0x40E7D2: GParted::BlockSpecialTest_NamedBlockSpecialObjectBySymlinkMatches_Test::TestBody() (test_BlockSpecial.cc:247)
      ...
      ==25110==  Address 0x1231ea93 is 115 bytes inside a block of size 32,816 free'd
      ==25110==    at 0x4C2ACBD: free (vg_replace_malloc.c:530)
      ==25110==    by 0x9F773AC: closedir (in /usr/lib64/libc-2.17.so)
      ==25110==    by 0x40E7AC: get_link_name (test_BlockSpecial.cc:153)
      ==25110==    by 0x40E7AC: GParted::BlockSpecialTest_NamedBlockSpecialObjectBySymlinkMatches_Test::TestBody() (test_BlockSpecial.cc:247)
      ...
      ==25110==  Block was alloc'd at
      ==25110==    at 0x4C29BC3: malloc (vg_replace_malloc.c:299)
      ==25110==    by 0x9F77280: __alloc_dir (in /usr/lib64/libc-2.17.so)
      ==25110==    by 0x40E746: get_link_name (test_BlockSpecial.cc:134)
      ==25110==    by 0x40E746: GParted::BlockSpecialTest_NamedBlockSpecialObjectBySymlinkMatches_Test::TestBody() (test_BlockSpecial.cc:247)
      ...
      [       OK ] BlockSpecialTest.NamedBlockSpecialObjectBySymlinkMatches (50 ms)
    
    Selected lines from test_BlockSpecial.cc:
    
      132  static std::string get_link_name()
      133  {
      134          DIR * dir = opendir( "/dev/disk/by-id" );
      ...
      141          bool found = false;
      142          struct dirent * dentry;
      143          // Silence GCC [-Wparentheses] warning with double parentheses
      144          while ( ( dentry = readdir( dir ) ) )
      145          {
      146                  if ( strcmp( dentry->d_name, "." )  != 0 &&
      147                       strcmp( dentry->d_name, ". " ) != 0    )
      148                  {
      149                          found = true;
      150                          break;
      151                  }
      152          }
      153          closedir( dir );
      154
      155          if ( found )
      156                  return std::string( "/dev/disk/by-id/" ) + dentry->d_name;
    
    So the memory referred to by dentry was allocated on line 134, freed on
    153 and accessed after freed on 156.
    
    Closes !41 - Fix test (dentry->d_name is invalidated by closedir...)
    cdba5cee