[FFmpeg-devel] [PATCH] mxfdec: make it work with other calling conventions

[Baptiste Coudurier]

On 06/30/2010 09:14 AM, M?ns Rullg?rd wrote:
> Baptiste Coudurier<baptiste.coudurier at gmail.com>  writes:
>
>> On 6/29/10 10:25 PM, Reimar D?ffinger wrote:
>>> On Tue, Jun 29, 2010 at 04:47:32PM -0700, Baptiste Coudurier wrote:
>>>> On 6/29/10 2:35 PM, Reimar D?ffinger wrote:
>>>>> On Tue, Jun 29, 2010 at 01:02:05PM -0700, Baptiste Coudurier wrote:
>>>>>> On 6/29/10 11:51 AM, Reimar D?ffinger wrote:
>>>>>>> On Tue, Jun 29, 2010 at 08:30:01PM +0200, Reimar D?ffinger wrote:
>>>>>>>> Hello,
>>>>>>>> currently mxfdec assumes that it can unpunished pass more arguments to functions
>>>>>>>> than they were declared with.
>>>>>>>> This is not true in general, in particular not for stdcall.
>>>>>>>> While I am not aware of any FFmpeg platform using it, IMHO this code is still
>>>>>>>> wrong.
>>>>>>>> Attached is a patch that fixes it, and I think it is not particularly bad.
>>>>>>>> It also fixes the last remaining warnings ("function declaration is not a prototype")
>>>>>>>> in that file.
>>>>>>
>>>>>> I don't have this warning.
>>>>>> gcc version 4.4.4 (Ubuntu 4.4.4-6ubuntu2)
>>>>>
>>>>> Right, I had been using -Wstrict-prototypes
>>>>>
>>>>>>> +#define METADATA_READ_FUNC(name) int name(void *arg, ByteIOContext *pb, int tag, int size, UID uid)
>>>>>>
>>>>>> I don't like the #define.
>>>>>
>>>>> Well, the alternative is having to change every single read function
>>>>> manually in case you'd ever need an additional parameter.
>>>>> And I found it very hard to find them.
>>>>
>>>> And when you add a new function, you have to look up for the define
>>>> to know which parameters are available. This is even more painful.
>>>
>>> Have you actually _tried_ changing the parameters of all those functions?
>>>
>>> I had to pick each single one from the table, search for it's
>>> declaration and then change it.  Searching for the define to get
>>> the argument names is quite simple in comparison.  But you have to
>>> maintain it, I'll change it if that's the only objection.
>>>
>>
>> I do know for sure that I've been working way more with that file than
>> you. Please remove the define.
>
> You still haven't fixed it.  If you don't commit a fix you're happy
> with within a few days, we'll go with Reimar's patch.  And once that
> is done, I *will* make prototype-less functions an error.
>

You will do no such thing without majority approval. You do not make the 
rules here, is that clear ?

-- 
Baptiste COUDURIER
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
FFmpeg maintainer                                  http://www.ffmpeg.org