[ogg-dev] [PATCH] skeleton.c

[Conrad Parker]

On 06/02/2008, ogg.k.ogg.k at googlemail.com <ogg.k.ogg.k at googlemail.com> wrote:
> I've had a second look, and I believe there really was a bug there,
> though my patch may not be optimal.
>
> As an example of an off by one bug:
>
> On the first run through the code, message_header_fields will be NULL,
> so _ogg_calloc will be called. Assume header_key and header_value
> are both "X", so strlen of each is 1. message_size will then by 6, and a
> block of 6 bytes is callocated.
> Then snprintf is called with a byte limit of message_size+1 (7, one more
> than the allocated size), with the string "X: X\r\n", 6 characters and a
> terminating NULL. 7 bytes will be written, overwrite.

yup, you're right. The (this_message_size+1) argument to snprintf,
after only allocating (this_message_size) chars, was asking for
trouble.

However if we simply truncate to the correct length then the final LF
is replaced with NUL by snprintf. So we need to allocate room for one
extra char (the NUL) as you have done, but then not truncate the
write: so we both allocate and print with length
(this_message_size+1).

Checked with valgrind, and committed in oggenc r14485 and liboggz r3448.

cheers,

Conrad.