[FFmpeg-devel] [PATCH] remove unused and broken test program in libavutil/base64.c

Stefano Sabatini stefano.sabatini-lala
Thu Feb 5 21:04:36 CET 2009 

On date Sunday 2009-02-01 23:33:08 +0100, Michael Niedermayer encoded:
> On Sun, Feb 01, 2009 at 07:06:21PM +0100, Stefano Sabatini wrote:
> > On date Saturday 2009-01-31 22:08:26 +0100, Michael Niedermayer encoded:
> > > On Sat, Jan 31, 2009 at 07:08:41PM +0100, Stefano Sabatini wrote:
> > > > On date Saturday 2009-01-31 01:11:00 +0100, Michael Niedermayer encoded:
> > > > > On Sat, Jan 31, 2009 at 12:26:20AM +0100, Stefano Sabatini wrote:
> > > > > > On date Friday 2009-01-30 17:06:22 +0100, Michael Niedermayer encoded:
> > > > > > > On Fri, Jan 30, 2009 at 12:46:36AM +0100, Stefano Sabatini wrote:
> > > > > > [...]
> > > > > > > > I think I addressed all the points.
> > > > > > > > 
> > > > > > > > Round 5:
> > > > > > > > ------------------8<----------------------8<-------------------------
> > > > > > > > #ifdef TEST
> > > > > > > > #include "mem.h"
> > > > > > > > #include "lfg.h"
> > > > > > > > 
> > > > > > > > #undef printf
> > > > > > > > 
> > > > > > > > #define MAX_DATA_SIZE    1024
> > > > > > > > #define MAX_ENCODED_SIZE 2048
> > > > > > > > 
> > > > > > > 
> > > > > > > > #define SHOW_STUFF(stuff) show_stuff ? (const char *)stuff : "[...]"
> > > > > > > 
> > > > > > > please explain wat this is supposed to be good for?
> > > > > > 
> > > > > > Yes, here there is the first lines of the output:
> > > > > > 
> > > > > > Encoding/decoding tests
> > > > > > Encoding data with size 0 bytes ''... encoded as ''... passed!
> > > > > > Encoding data with size 1 bytes '1'... encoded as 'MQ=='... passed!
> > > > > > Encoding data with size 2 bytes '22'... encoded as 'MjI='... passed!
> > > > > > Encoding data with size 3 bytes '333'... encoded as 'MzMz'... passed!
> > > > > > Encoding data with size 4 bytes '4444'... encoded as 'NDQ0NA=='... passed!
> > > > > > Encoding data with size 5 bytes '55555'... encoded as 'NTU1NTU='... passed!
> > > > > > Encoding data with size 6 bytes '666666'... encoded as 'NjY2NjY2'... passed!
> > > > > > Encoding data with size 7 bytes 'abc:def'... encoded as 'YWJjOmRlZg=='... passed!
> > > > > > Encoding data with size 0 bytes '[...]'... encoded as '[...]'... passed!
> > > > > > Encoding data with size 968 bytes '[...]'... encoded as '[...]'... passed!
> > > > > > Encoding data with size 780 bytes '[...]'... encoded as '[...]'... passed!
> > > > > > Encoding data with size 647 bytes '[...]'... encoded as '[...]'... passed!
> > > > > > ...
> > > > > > 
> > > > > > For the data with a reference encoded string I think it is a good idea
> > > > > > to show how the data is encoded, in case of problem the hacker should
> > > > > > be able to immediately spot where the problem is.
> > > > > > 
> > > > > > For the other ones the length of the encoded string is usually too
> > > > > > long and it is too awkward to display it, but no strong opinion on
> > > > > > this, just tell how do you prefer it.
> > > > > 
> > > > > hmm ok, ive not conciously registererd that this was run with several hundread
> > > > > bytes.
> > > > > >  
> > > > > > > [...]
> > > > > > > >     printf("\nDecoding tests on invalid data\n");
> > > > > > > >     {
> > > > > > > >         uint8_t data[32];
> > > > > > > >         const char *encoded[] = { "M", "M=M=", "MQ===" };
> > > > > > > 
> > > > > > > why is this splited off and not in the main table?
> > > > > > 
> > > > > > I believe this test cannot be reduced to the previous one, here we're
> > > > > > just *decoding* and checking for validity.
> > > > > 
> > > > > if(A) A -> decode, cmp B
> > > > > if(B) B -> encode, cmp A
> > > > >
> > > > > B being NULL
> > > > >
> > > > > am i missing something? 
> > > > 
> > > > The test is an encoding/decoding test, not a decoding/encoding test,
> > > > we could maybe find some trick to make encode_decode_test() act as a
> > > > decoding invalidity test, but this:
> > > > 
> > > > 1) would increase complexity of a program meant to be simple
> > > > 2) it's not worth the trouble
> > > 
> > > a if(B) is hardly more complex than to duplicate the whole check
> > 
> > Well we have:
> > test_encode_decode:
> > D -> E, cmp (E, E'), E -> D', cmp (D, D')
> > 
> > the second test is:
> > E -> D, if decode successfull -> fail
> 
> if(D){
>     D->E'
>     if(cmp(E, E'))
>         fail
> }
> 
> E->D'
> if(cmp(D, D'))
>     fail
> 
> cmp(A,B){
>     if((!A) != (!B)) return -1
>     else if(A)       return strcmp(A,B)
> }
> 
> anyway if this turns out more complex, forget it

Yes, I'd prefer to keep the current solution, which I find simpler and
possibly more readable.
 
> > I cannot see an immediate way to reduce the second test to the first
> > one, if there is I think it would most likely make the code more
> > complex, maybe I'm wrong.
> > 
> > > also about SHOW_STUFF() it shouldnt be there and you shouldnt print
> > > anything at all if things are ok, just print what does not produce the
> > > expected output or an OK at the end
> > > if a 1kb case mismatches it should be printed as well
> > 
> > OK, done, but printing data will show non-printable chars, is this a
> > problem?
> 
> well, of course, printing hex maybe better for debuging anyway.

Yes. And while we're at it what about to move av_hex_dump() from lavf
to lavu?

That's a nice function to have in libavutil, in this case it would
have saved its reimplementation in this test program.
 
> also i think we are wasting time on this rather pointless piece of code
> i guess thats my fault :(

I enjoy your remarks most of the time, but I agree there are more
urgent/interesting problems to solve out there! ;-)

> > > > > [...]
> > > > > > -- 
> > > > > > FFmpeg = Fostering and Fundamental Murdering Peaceful Educated Guru
> > > > > 
> > > > > are you aware that some of these are semantically contradictionary?
> > > > 
> > > > Yes, and FFmpeg itself has some internal contradictions ;-),
> > > > furthermore semantic checkers are a little expensive these days...
> > > 
> > > really?
> > > 
> > > download some book in ascii txt format
> > > grep for a word starting with f
> > > then grep for "<1word> f" 
> > > then grep for "<2word> m" 
> > > then grep for "<3word> p" 
> > > ...
> > > 
> > > the result should be less boring i hope :)
> > > at least each consecutive word pair would be semantically valid even if
> > > longer chains may not be
> > 
> > Well, I followed your suggestion (check the attached script),
> > unfortunately is not easy to find such a words chain even in large
> > books, for example check this:
> 
> hmm seems you are not completely wrong, i also dont end up with too
> good ones

[...]

:)

Regards.
-- 
FFmpeg = flee from my pity even Gandalf
-------------- next part --------------
A non-text attachment was scrubbed...
Name: add-new-base64-test.patch
Type: text/x-diff
Size: 4363 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090205/ac436e87/attachment.patch>

More information about the ffmpeg-devel mailing list