[FFmpeg-devel] [patch] libswscale: fix warning regarding "no return in non void function"

[Michael Niedermayer]

On Tue, Apr 27, 2010 at 12:58:45AM +0300, Dominique Leuenberger wrote:
> Quoting Michael Niedermayer <michaelni at gmx.at>:
>
>> On Mon, Apr 26, 2010 at 05:23:56PM +0300, Dominique Leuenberger wrote:
>>> Quoting Dominique Leuenberger <dominique-ffmpeg-devel at leuenberger.net>:
>>>
>>>> In libswscale, in case HAVE_7REGS is not defined to true, there are  two
>>>> functions that result in empty code blocks.
>>>>
>>>> And as a consequence, the compiler warns about no return value in a  non
>>>> void function.
>>>>
>>>> The attached patch fixes this by:
>>>> - Moving the #if HAVE_7REGS out of the function declaraion (meaning  the
>>>> function does not exist at all if we do not HAVE_7REGS.
>>>> - Change some logic to not call the functions if they would not return
>>>> useful result anyway.
>>>
>>> Just to bring it back to life:
>>> is a compiler warning about an empty function block 'cleaner' than 
>>> avoiding
>>> this using an #if directive?
>>
>> a #if* is always worse than a warning
>
> Any reference that staes that other than your own opinion?

iam sure you can find other people with similar oppinions,
iam sure you can find ones with differing oppinion.
they arent maintainer of this file though

that said, i agree with mans that there likely is a way to get rid of
the warning without am additional #if


>
>>>
>>> As said: we do have build log checks that fail in various cases known to
>>> cause program errors: one of them is no return in non-void function 
>>> (which
>>> happens to be triggered in this specific case).
>>>
>>> Yes, I agree, the compiler might get it wrong. But why not tell the
>>> compiler that we're smarter? Another day somebody might call the function
>>> without if (HAVE_7REGS), and then at least the compiler knows this is not
>>> supposed to be and strictly tells: function is not defined.
>>
>> theres no reason to belive that this would ever be done
>> but we know the #if will make the code less readable, thats a certain
>> problem vs. a unlikely problem
>
> Sure: we also never had a reason to believe earth was not a plate and we 
> would not fall off at the edge.

we had good reason to belive so. The earth does from the view of a person
of prehistoric times look locally flat, its reasonable to assume its globally
flat from this. Only once the curvature was actually observed (though ships
and later through exact measurement of shadows at 2 places at exactly the same
time) was there reason to assume it was not flat.



>
> Or are you trying to me that you have troubles understanding an #if when 
> you see it?

look at glibc sources and headers


>
> I'm not in favor of replacing them all (agreed, that would make code 
> unreadable.. that's m y personal opinion and can be quoted) but putting 
> them in places where they do make sense. After all, there seemed not to be 
> any objection of adding the #if HAVE_7REGS in the _template.c file. Why not 
> be consistent and do it in both files (that actually belong together) ? 

its in _template likely because gcc fails to compile it without it


> doing twice the same in different manner also does not increase readability 
> of code.

doing twice the same in 2 different files is unstable, its just a mater of
time until one is changed with the other forgotten

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When the tyrant has disposed of foreign enemies by conquest or treaty, and
there is nothing more to fear from them, then he is always stirring up
some war or other, in order that the people may require a leader. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100427/81ecad76/attachment.pgp>