[Ffmpeg-devel] [PATCH] Wildcard and catalog image sequences

[Michael Niedermayer]

Hi

On Wed, Aug 30, 2006 at 02:00:38PM +0200, Michel Bardiaux wrote:
> Michael Niedermayer wrote:
> >Hi
> >
> >On Wed, Aug 30, 2006 at 12:38:30PM +0200, Michael Niedermayer wrote:
> >[...]
> >>>>>+static void free_catalog(int last_index, char*** pindex_vector)
> >>>>>+{
> >>>>>+    int i;
> >>>>>+    char** index_vector = *pindex_vector;
> >>>>>+    for(i=0;index_vector&&i<=last_index;i++)
> >>>>>+        av_free(index_vector[i]);
> >>>>>+    av_free(index_vector);
> >>>>>+    *pindex_vector = NULL;
> >>>>av_freep()
> >>>>and the index_vector variable seems unneeded
> >>>Wilco, but my code was actually correct. It seems to me you *require* 
> >>>compact code and are ready to reject a patch simply because you find the 
> >>>coding style too verbose for your taste!
> >>yes, code must be simple, patches must be minimal, no superfluous changes
> >
> >the coding rules also say that:
> >"Main priority in FFmpeg is simplicity and small code size (=less bugs). "
> >this has been written by fabrice IIRC and was there since a very long time
> >
> >[...]
> 
> I would dispute that a 4-liner instead of a 1-liner using ?: generates 
> more object code; and it does not seem *simpler* to me. I think you tend 
> too much towards compactness even at the price of obfuscation. But, OK, 
> I'll try to respect your wishes.

looking at for example:

>>+int filename_catalog_test(const char *filename)
>>+{
>>+    if(!filename)
>>+        return (-1);
>>+    else if(filename[0]=='@')
>>+        return 0;
>>+    else
>>+        return (-1);

versus

>return filename && filename[0]=='@';

i cant see how the later is more obfuscated, additionally
1. is the NULL check really needed at all?
2. in C 0 is false not 0 is true, using 0 as true is broken as logical and/or
   will no longer have their intuitive meaning, do you disagree here?

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In the past you could go to a library and read, borrow or copy any book
Today you'd get arrested for mere telling someone where the library is