[flac-dev] num_comments==0 and comments==0

[Brian Willoughby]

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.mail at 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>