[FFmpeg-devel] [PATCH] Improve documentation for libavutil/base64.h
Stefano Sabatini
stefano.sabatini-lala
Sun Feb 8 17:19:50 CET 2009
On date Sunday 2009-02-08 13:10:56 +0100, Michael Niedermayer encoded:
> On Sun, Feb 08, 2009 at 11:31:48AM +0100, Stefano Sabatini wrote:
[...]
> > Since we can express the constraint with C semantic in the base64.c
> > code, then I think there is no point into keeping the warning notice,
> > please check again.
> >
> > Last patch makes the implementation params consistent with those in
> > the declaration, also I think it improves readability, but I won't
> > push on it if you think is overkill.
> >
> > Regards.
> > --
> > FFmpeg = Fundamental and Fast Mournful Pitiful Experimenting Gem
>
> > Index: libavutil/base64.c
> > ===================================================================
> > --- libavutil/base64.c (revision 17046)
> > +++ libavutil/base64.c (working copy)
> > @@ -79,7 +79,7 @@
> > int bytes_remaining = len;
> >
> > if (len >= UINT_MAX / 4 ||
> > - buf_len < len * 4 / 3 + 12)
> > + buf_len < (len+2) / 3 * 4 + 1)
> > return NULL;
> > ret = dst = buf;
> > while (bytes_remaining) {
>
> iam still waiting for a proof that it is large enough, and yes i do not
> consider "the ffmpeg leader said its likely ok a valid proof", this is
> security relevant and the 10 bytes this safes on one hand is vs. possibly
> thousands of users having their system hacked if we did miss something.
>
> just write a 2 line loop that check sizes from 0..X and checks that the
> last write was inside the buffer.
Sure.
-------------------------8<--------------------------------------------
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <libavutil/base64.h>
#define MAX_BUF_SIZE 1024 * 1024
int main(void)
{
char encoded[MAX_BUF_SIZE];
char data [MAX_BUF_SIZE];
int i, data_size;
for (data_size=0; data_size<100000; data_size++) {
int encoded_size = (data_size + 2) / 3 * 4 + 1;
if(!av_base64_encode(encoded, MAX_BUF_SIZE, data, data_size))
printf("failed!\n");
else
printf("%d\t%d\t%d\n", data_size, encoded_size, strlen(encoded) + 1);
}
return 0;
}
-------------------------8<--------------------------------------------
the strlen of the encoded buffer results always equal to the value
computed in encoded_size.
And now a math proof, for what are worth my demonstrations:
L' = smallest multiple of 3 equal to or greater than L
L' = floor((L+2)/3) * 3
So we have:
4/3 * L' = 4/3 * floor((L+2) / 3) * 3 =
= 4 * floor((L+2) / 3) * 3 / 3 =
= 4 * floor((L+2) / 3)
which translated in C semantics is:
(L+2) / 3 * 4;
> > Index: libavutil/base64.h
> > ===================================================================
> > --- libavutil/base64.h (revision 17046)
> > +++ libavutil/base64.h (working copy)
> > @@ -24,16 +24,26 @@
> > #include <stdint.h>
> >
> > /**
> > - * Decodes Base64.
> > - * Parameter order is the same as strncpy().
> > + * Decodes the base64-encoded string in \p src and puts the decoded
> > + * data in \p dst.
> > + *
> > + * @param dst_size size in bytes of the \p dst buffer, it should be at
> > + * least 3/4 of the length of \p src
> > + * @return the number of bytes written, or a negative value in case of
> > + * error
> > */
> > -int av_base64_decode(uint8_t * out, const char *in, int out_length);
> > +int av_base64_decode(uint8_t *dst, const char *src, int dst_size);
>
> addition of doxy, ad renamig params should be in seperate patches.
>
>
> [...]
>
> > Index: ffmpeg/libavutil/base64.c
> > ===================================================================
> > --- ffmpeg.orig/libavutil/base64.c 2009-02-08 11:11:29.000000000 +0100
> > +++ ffmpeg/libavutil/base64.c 2009-02-08 11:18:30.000000000 +0100
> > @@ -42,25 +42,25 @@
> > 0x2c, 0x2d, 0x2e, 0x2f, 0x30, 0x31, 0x32, 0x33
> > };
> >
> > -int av_base64_decode(uint8_t * out, const char *in, int out_length)
> > +int av_base64_decode(uint8_t *dst, const char *src, int dst_size)
>
> i dont see the point of this rename.
>
> [...]
> > -char *av_base64_encode(char * buf, int buf_len, const uint8_t * src, int len)
> > +char *av_base64_encode(char *dst, int dst_size, const uint8_t *src, int src_size)
> > {
>
> len vs size
> buf vs dst vs out
> len vs src len
> are seperate changes
>
> iam in favor of adding the src to len to make it clearer
> iam also in favor of using in/out or if you prefer b64/bin names
New series attached:
* base64-relax-constr.patch
Relax constraints required for the output buffer size of av_base64_encode().
* base64-size-vs-len.patch
Consistently prefer size vs len/length, length is an ambiguous term
since it can be associated to the length of a string in the strlen()
sense.
* base64-in-out-vs-src-buf.patch.
For consistency/readability sake, also out is more meaningful than
buf.
* base64-in-size-vs-size.patch
Prefer in_size against size in av_base64_encode(), for readability reason.
* base64-space-nits.patch.
Apply spacing nits.
* base64-doxy.patch
Gloriously document the base64 interface.
Regards.
--
FFmpeg = Foolish Fostering Moronic Puritan Esoteric Geek
-------------- next part --------------
A non-text attachment was scrubbed...
Name: base64-relax-constr.patch
Type: text/x-diff
Size: 470 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090208/58090cee/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: base64-size-vs-len.patch
Type: text/x-diff
Size: 2304 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090208/58090cee/attachment-0001.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: base64-in-out-vs-src-buf.patch
Type: text/x-diff
Size: 1592 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090208/58090cee/attachment-0002.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: base64-in-size-vs-size.patch
Type: text/x-diff
Size: 1561 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090208/58090cee/attachment-0003.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: base64-space-nits.patch
Type: text/x-diff
Size: 979 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090208/58090cee/attachment-0004.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: base64-doxy.patch
Type: text/x-diff
Size: 1222 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090208/58090cee/attachment-0005.patch>
More information about the ffmpeg-devel
mailing list