Re: [flac-dev] Memory leaks
lvqcl wrote: If realloc fails, then the previous value of pointer x is lost and we have memory leak. The simplest fix is to add new functions like this: static inline void *realloc_noleak_(void *ptr, size_t size) Actually it won't fix this code in FLAC__metadata_object_vorbiscomment_resize_comments(): else if(0 == (object-data.vorbis_comment.comments = realloc(object-data.vorbis_comment.comments, new_size))) return false; because memory from object-data.vorbis_comment.comments[i].entry will leak. ___ flac-dev mailing list flac-dev@xiph.org http://lists.xiph.org/mailman/listinfo/flac-dev
Re: [flac-dev] num_comments==0 and comments==0
What is the advantage of removing an assert? - even FLAC_ASSERT() My understanding is that assert() is only compiled into the code with Debug builds, whereas with a Release build the assert() macro will generate no code at all. In other words, when you build for testing, the assert is there, but when you build the fully optimized version the assert will be removed anyway. Seems safer to leave the FLAC_ASSERT() and then work to make sure the tests cover the potential memory leaks as well as ensure that fully-optimized release builds turn off all assert() macros. Brian Willoughby Sound Consulting p.s. I haven't looked too closely at this specific commit change, but speak from general experience. On Jul 4, 2015, at 4:30 AM, lvqcl lvqcl.m...@gmail.com wrote: About the removed assert in this commit: http://git.xiph.org/?p=flac.git;a=commitdiff;h=bc5113007a53be2c621d5eb5f4485eddf947ef37 It looks reasonable that if x.num_comments == 0 then x.comments is also NULL. Otherwise there's probably a leak somewhere that should be fixed. I found several places where the situation is reverse: comments can be 0 but num_comments is not; IMHO it makes sense to fix them even if there are no crashes (yet?). The patch is attached (of course it can't fix the removed assert because it doesn't fix any leak).num_comments.patch ___ flac-dev mailing list flac-dev@xiph.org http://lists.xiph.org/mailman/listinfo/flac-dev
Re: [flac-dev] num_comments==0 and comments==0
lvqcl wrote: It looks reasonable that if x.num_comments == 0 then x.comments is also NULL. Otherwise there's probably a leak somewhere that should be fixed. Now I'm not sure about this, maybe it's wrong, and in this case comments is just an array with 0 (==num_comments) valid elements. ___ flac-dev mailing list flac-dev@xiph.org http://lists.xiph.org/mailman/listinfo/flac-dev
[flac-dev] Memory leaks
There are several places in libFLAC like this:
if(0 == (x = realloc(x, size)))
return false;
and
if(0 == (x = safe_realloc_mul_2op_(x, size1, size2))) {
decoder_state = FLAC__STREAM_DECODER_MEMORY_ALLOCATION_ERROR;
return false;
}
If realloc fails, then the previous value of pointer x is lost and we have
memory leak. The simplest fix is to add new functions like this:
static inline void *realloc_noleak_(void *ptr, size_t size)
{
void *tmp = realloc(ptr, size);
if(!tmp)
free(ptr); /* no memory leak */
return tmp;
}
static inline void *safe_realloc_mul_2op_noleak_(void *ptr, size_t size1,
size_t size2)
{
if(!size1 || !size2)
return realloc(ptr, 0); /* preserve POSIX realloc(ptr, 0)
semantics */
if(size1 SIZE_MAX / size2)
return 0;
return realloc_noleak_(ptr, size1*size2);
}
And use them in such places. Or maybe some better solution exists?
___
flac-dev mailing list
flac-dev@xiph.org
http://lists.xiph.org/mailman/listinfo/flac-dev
[flac-dev] num_comments==0 and comments==0
About the removed assert in this commit: http://git.xiph.org/?p=flac.git;a=commitdiff;h=bc5113007a53be2c621d5eb5f4485eddf947ef37 It looks reasonable that if x.num_comments == 0 then x.comments is also NULL. Otherwise there's probably a leak somewhere that should be fixed. I found several places where the situation is reverse: comments can be 0 but num_comments is not; IMHO it makes sense to fix them even if there are no crashes (yet?). The patch is attached (of course it can't fix the removed assert because it doesn't fix any leak). num_comments.patch Description: Binary data ___ flac-dev mailing list flac-dev@xiph.org http://lists.xiph.org/mailman/listinfo/flac-dev
