[FFmpeg-devel] [PATCH] lavu: add av_bprintf and related.

[Nicolas George]

Le duodi 22 ventôse, an CCXX, Stefano Sabatini a écrit :
> nit+++: maybe len_add to stress the fact that it represents a length

I'll try not to forget to change it.

> > +        room = av_bprint_room(buf);
> > +        dst = room ? buf->str + buf->len : NULL;
> > +        va_start(vl, fmt);
> > +        add = vsnprintf(dst, room, fmt, vl);
> 
> > +    /* arbitrary margin to avoid small overflows */
> > +    add = FFMIN(add, UINT_MAX - 5 - buf->len);
> 
> I'm not sure I understand the need for this check.
> 
> room provides the available size left in the buffer, since the max
> size is UINT_MAX then we're sure that:
> len+room <= UINT_MAX
> 
> and so since add+1 <= room then:
> len+add+1 <= UINT_MAX.

I think you are missing the fact that the return value of snprintf is the
length of the string that would have been written if the buffer had been
large enough.

In other words, snprintf(buf, 5, "Hello World!") returns 12, not 4 or 5.

Therefore, since add can be greater than room, len+add can overflow.

Furthermore, I want the user to be able to be able to write "len+3", for
small values of 3, without bothering to check for overflow: 5 is a nice
margin: a 4-bytes length field plus the string plus a 0-terminator.

Of course, all this is purely theoretical: in practice, I do not expect any
strings with this API to reach even UINT_MAX/10.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120311/61abe1c0/attachment.asc>