Hi all, I'm the author of flactag, a utility for tagging whole-album FLAC files with embedded CUE sheets using data from the MusicBrainz servers. I've recently run it through valgrind, and I'm seeing memory leaks like the following: 12 bytes in 1 blocks are definitely lost in loss record 1 of 5 at 0x402377E: operator new(unsigned) (vg_replace_malloc.c:224) by 0x41448A8: FLAC::Metadata::local::construct_block(FLAC__StreamMetadata*) (in /usr/lib/libFLAC++.so.6.2.0) by 0x41455D7: FLAC::Metadata::Iterator::get_block() (in /usr/lib/libFLAC++.so.6.2.0) by 0x8072CB5: CFlacInfo::Read() (FlacInfo.cc:154) by 0x8054BF4: CFlacTag::LoadData() (flactag.cc:543) by 0x8058D45: CFlacTag::CFlacTag(CCommandLine const&) (flactag.cc:134) by 0x805D4E2: main (flactag.cc:66) In my code, the offending line is: m_PictureBlock=(FLAC::Metadata::Picture*)Iterator.get_block(); (m_PictureBlock is a pointer to a FLAC::Metadata::Picture object). The documentation seems to imply that you don't need to delete the pointer that is returned here, but the memory leak report seems to contradict that. Can anyone tell me if I should be deleteing this point? Or is there something else I should be doing to clean up? Thanks Andy
> I've recently run it through valgrind, and I'm seeing memory leaks like the > following: > > 12 bytes in 1 blocks are definitely lost in loss record 1 of 5 > at 0x402377E: operator new(unsigned) (vg_replace_malloc.c:224) > by 0x41448A8: FLAC::Metadata::local::construct_block(FLAC__StreamMetadata*) > (in /usr/lib/libFLAC++.so.6.2.0) ? by 0x41455D7: > FLAC::Metadata::Iterator::get_block() (in /usr/lib/libFLAC++.so.6.2.0) > by 0x8072CB5: CFlacInfo::Read() (FlacInfo.cc:154) > by 0x8054BF4: CFlacTag::LoadData() (flactag.cc:543) > by 0x8058D45: CFlacTag::CFlacTag(CCommandLine const&) (flactag.cc:134) > by 0x805D4E2: main (flactag.cc:66) > > In my code, the offending line is: > > m_PictureBlock=(FLAC::Metadata::Picture*)Iterator.get_block();I've never used the C++ API, but the source for get_block() is just: return local::construct_block(::FLAC__metadata_simple_iterator_get_block(iterator_)); and construct_block() looks like (simplified for the picture case): Prototype *ret = 0; ret = new Picture(object, /*copy=*/false); return ret; So I don't see any way this couldn't be a leak if you aren't deleting the pointer returned from get_block(). The documentation says: * The ownership of pointers in the C++ layer follows that in * the C layer, i.e. * - The objects returned by get_block() are yours to * modify, but changes are not reflected in the FLAC file * until you call set_block(). The objects are also * yours to delete; they are not automatically deleted * when passed to set_block() or insert_block_after(). So I think it will be necessary for you to delete the object manually. Stephen
Hi,
In article <BANLkTi=L+XNe3zVRoVMzSYvCwVhtpNh7xQ at mail.gmail.com>,
Stephen F. Booth<me at sbooth.org>
wrote:> I've never used the C++ API, but the source for get_block() is just:
>
> return
local::construct_block(::FLAC__metadata_simple_iterator_get_block(iterator_));
>
> and construct_block() looks like (simplified for the picture case):
>
> Prototype *ret = 0;
> ret = new Picture(object, /*copy=*/false);
> return ret;
>
> So I don't see any way this couldn't be a leak if you aren't
deleting
> the pointer returned from get_block().
>
> The documentation says:
>
> * The ownership of pointers in the C++ layer follows that in
> * the C layer, i.e.
> * - The objects returned by get_block() are yours to
> * modify, but changes are not reflected in the FLAC file
> * until you call set_block(). The objects are also
> * yours to delete; they are not automatically deleted
> * when passed to set_block() or insert_block_after().
>
> So I think it will be necessary for you to delete the object manually.
Thanks for that. Can't find all of that in the documentation I'm looking
at:
http://flac.sourceforge.net/api/index.html
However, it does all make sense and ties in with my reading of the library
source too.
Thanks a lot for the response.
Andy