Re: [Libcdio-devel] [RFC] New API iso9660_statv2_t as API/ABI compatible way to read files >= 4 GiB

[Thomas Schmitt]

Hi,

sorry for not yet providing the branch for the problem with iso9660_private.h.
The C++ interface to iso9660 kept me busy.
  
http://git.savannah.gnu.org/cgit/libcdio.git/commit/?h=ts-multiextent&id=77ae08430b1917956ddfe00a9fec10d1c8759470


I still have some problems of understanding with class Stat of
  include/cdio++/iso9660.hpp
as it is in the master branch.

To me it looks like
* a wrapper around a pointer to iso9660_stat_t.
* the owner of the memory that is pointed to. At least ~Stat() disposed it.
* base of a vector type.

Nevertheless
* some methods seem to simply copy the pointer without making precautions
  so that the iso9660_stat_t object cannot be freed twice.
* method .clear() of the derived vector type of Stat does not dispose
  its Stat elements and their iso9660_stat_t objects.

-----------------------------------------------------------------------

The methods

    Stat(const Stat& copy_in)
    const Stat& operator= (const Stat& right)

simply copy pointers, if i am not totally mistaken:

    p_stat = copy_in.p_stat;
    this->p_stat = right.p_stat;

The definition of class member p_stat is

    iso9660_stat_t *p_stat;


Am i wrong ? Does it do the equivalent of memcpy() with the objects
that are pointed to ?

If i am wrong, there remains the problem that iso9660_stat_t cannot be
simply copied at the risk of creating a duplicate pointer to calloc memory
by iso9660_stat_t.rr.psz_symlink .

If i am right, then already the .p_stat members are duplicate pointers
and memory gets corrupted if the one is used after the other was disposed.
(I am not sure whether those methods are actually in use in the
 example/C++/OO/*iso* programs.)

-----------------------------------------------------------------------

After keeping IFS.readdir() from destroying the iso9660_stat_t objects
before the C++ code can use them via stat_vector_t, there remains a fat
memory leak, because this call

  stat_vector.clear();

in example/C++/OO/isolist.cpp does not call the destructor of the Stat
vector elements. (That would be ~Stat().)

The vector type is defined for class Stat by
  typedef vector< ISO9660::Stat *> stat_vector_t;

I read in
  http://www.cplusplus.com/reference/vector/vector/clear/
  "void clear();
   Removes all elements from the vector (which are destroyed), leaving
   the container with a size of 0.
  "
Doesn't "are destroyed" mean that the element destructors are called ?

example/C++/OO/iso4.cpp seems to expect no deletion of elements, as it
disposes the Stat objects already in the loop that iterates over the
vector that came from FS.readdir().
(I now do the same in isolist.cpp. So valgrind is happy.)


Do we have a C++ expert who can clarify whether vector.clear() is supposed
to call the destructors of its elements ?

(The memory corruption and the subsequent leak after correction can be
 seen by
   valgrind --leak-check=full example/C++/OO/isolist "$any_iso_image"
)

-----------------------------------------------------------------------

Have a nice day :)

Thomas