[FFmpeg-devel] [PATCH] Improve documentation for libavutil/base64.h
Tue Jan 27 00:45:31 CET 2009
On date Sunday 2009-01-25 22:51:53 +0100, Michael Niedermayer encoded:
> On Sun, Jan 25, 2009 at 10:37:07PM +0100, Stefano Sabatini wrote:
> > On date Sunday 2009-01-25 18:10:05 +0100, Michael Niedermayer encoded:
> > > On Sun, Jan 25, 2009 at 02:04:52AM +0100, Stefano Sabatini wrote:
> > [...]
> > > > Can you confirm that the attached patch is correct? (At least I can
> > > > confirm that it works with the test program.)
> > >
> > > I can confirm that
> > > * it could lead to a sechole if you missed something and i suspect you did
> > > * its more complex
> > > * you did not list any advantage of this change
> > Use the minimal size data required. But I'll check better for the
> > first point.
> > [...]
> > > > /**
> > > > - * encodes base64
> > > > - * @param src data, not a string
> > > > - * @param buf output string
> > > > + * Encodes in base-64 the data in \p src and puts the resulting string
> > > > + * in \p dst.
> > > > + *
> > > > + * @param dst_len lenght of the \p dst string, it has to be at least
> > > > + * 4/3 * N, where N is the smaller multiple of 3 greater than or equal
> > > > + * to \p src_size.
> > >
> > > this is even worse, you AGAIN document the current implementation limit
> > > but now its much more complex and even exploitable
> > Honestly, I cannot see your point here.
> > We have a function which fails if the buffer where to write cannot
> > contain the data to write into it.
> > We know which is the size of this data, so we have two choices:
> > 1) we can fail if the data size provided by the user is not big
> > enough. In this case the documentation should give some hints
> > regarding the size of the buffer to be provided, ideally this
> > should be the minimal required, and this value shouldn't depend on
> > the implementation, for this reason it looks safe to mention it in
> > the docs.
> > 2) we can write in the buffer for its whole size, avoiding to go
> > beyond its size. The problem with this is that the user isn't able
> > to detect the failure, that's not a big problem since he can
> > compute the required size by its own.
> > Problem with the actual implementation is that it requires an
> > (apparently, but maybe I'm wrong) arbitrary data size of len * 4 / 3 +
> > 12, so an user providing a buffer with the minimal required size will
> > fail.
> Well my point is the user should neither depend on the implementation nor
> should he need precisse knowledge of base64 coding, and a len%3+1-len/123+98
> is a little much for just avoiding 1 or 2 bytes overallocation
> anyway my wild guess is that (len+2)/3*4+1 might be minimal
floor (4/3 * (L + 2)) = floor (4/3 * (L' + R + 2)) =
= floor (4/3 * (L'' + R') =
= floor (4/3 * L'' + floor( 4/3 * R') =
= 4/3 * L'' + R'
L' being the smaller multiple of 3 not greater than L, R = L - L',
L'' being L' + 3, R' = L' + R + 2 - L''
in the worst case L' + R = L'', R' = 2, so we have two bytes
overallocated, which is better than 12.
Also you can demonstrate in this case that 4/3 * L + 2 = 4/3 * (L + 2).
> > But I'll check better the code and the spec, in the case I didn't
> > considered some special case.
> in your last attempt you at least seem to have missed the 0 byte at the end
No I was considering len as a string length (in the sense of strlen())
rather than a buffer size.
I hope it is be more clear in the patch below.
FFmpeg = Forgiving and Forgiving Minimalistic Picky Extended Game
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 1588 bytes
Desc: not available
More information about the ffmpeg-devel