[FFmpeg-devel] [PATCH] OpenEXR decoder rev-12

[Reimar Döffinger]

On Wed, Jul 29, 2009 at 02:39:08PM +0200, Jimmy Christensen wrote:
>> That's what functions are there for. I think in quite a few places readability
>> could be improved quite a bit by creating a well named function, but I think
>> you so far ignored all my suggestions in that regard.
>>
>
> It's not as much as I ignored them. I tried making them into functions  
> but since it needs to exit with a return -1, I didn't know how to make  
> the function make the main function return -1.

E.g.
If (get_value(...) < 0)
    return -1;
That still duplicates the if, but should still be less code, at least
the av_log is centralized.
You could then consider if its reasonable to wrap even that into a macro.

>> Pointless () around buf_end - buf, also I still think this check is duplicated
>> all over the place and could be factored out at worst by using a macro.
>
> AFAIK Michael Niedermayer hates macros :)

I don't think so at all. But macros avoid code duplication only at the source
level, not the code level and have some usability issues that make a static inline
function often preferable over a macro (or for non-speed-critical code like here
even without the inline).