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

[Måns Rullgård]

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.

-- 
M?ns Rullg?rd
mans at mansr.com