ogg dev - Jul 2009 - Fixing ogg vorbis corruption caused by bad metadata

Erik de Castro Lopo <mle+la at mega-nerd.com> wrote:
> Martin Leese wrote:
>> Anyway, calling alloc()s with no corresponding
>> free()s is a memory leak.  Not good code.
>
> The alloca() function allocates space on the stack and
> that allocation is automatically freed when the function
> that did the allocation returns.
>
> The Linux man page is quite informative.
Yes, but is your code intended to work only
under Linux, and nowhere else?  What
matters is what the C manual says, and C still
has no automatic garbage collection.  It looks
like Microsoft currently supports C90 (with only
some of C99).

My ANSI C manual is old (1988), so things will
have changed, but in my copy alloc() is paired
with afree() and, further, calls to afree() must
be made in the opposite order to the calls
made on alloc().  (In my manual, malloc() is
paired with free(), and the order of the free()s
is unrestricted.)

Hopefully, things are not so restricted
nowadays.

Regards,
Martin
-- 
Martin J Leese
E-mail: martin.leese  stanfordalumni.org
Web: http://members.tripod.com/martin_leese/

On Wed, Jul 22, 2009 at 7:24 PM, Martin
Leese<martin.leese at stanfordalumni.org> wrote:
> Erik de Castro Lopo <mle+la at mega-nerd.com> wrote:
>
>> Martin Leese wrote:
>>> Anyway, calling alloc()s with no corresponding
>>> free()s is a memory leak.  Not good code.
>>
>> The alloca() function allocates space on the stack and
>> that allocation is automatically freed when the function
>> that did the allocation returns.
>>
>> The Linux man page is quite informative.
>
> Yes, but is your code intended to work only
> under Linux, and nowhere else?  What
> matters is what the C manual says, and C still
> has no automatic garbage collection.  It looks
> like Microsoft currently supports C90 (with only
> some of C99).
I appear to be spouting nonsense again.  I
should know better than to offer advice about
coding when I have been out of it for so long.

However, there is a general point.  Xiph policy
now encourages cover art in VorbisComments
using the METADATA_BLOCK_PICTURE tag,
visit:
http://wiki.xiph.org/VorbisComment#Cover_art

This means that VorbisComments can be
huge, and so memory for them now needs to
be allocated from the heap, not the stack.

Regards,
Martin
-- 
Martin J Leese
E-mail: martin.leese  stanfordalumni.org
Web: http://members.tripod.com/martin_leese/

> However, there is a general point. ?Xiph policy
> now encourages cover art in VorbisComments
> using the METADATA_BLOCK_PICTURE tag,
> visit:
> http://wiki.xiph.org/VorbisComment#Cover_art
>
> This means that VorbisComments can be
> huge, and so memory for them now needs to
> be allocated from the heap, not the stack.
Huge is relative.  Are people really sticking multi-megabyte binary
comments at the head of Vorbis files?  Ugh.

The code should be updated if for no other reason that this doesn't
get blamed on us :-(

Monty