[FFmpeg-devel] [PATCH] remove useless mm.c close function

[Michael Niedermayer]

On Sun, Aug 24, 2008 at 03:28:21PM +0100, M?ns Rullg?rd wrote:
> Michael Niedermayer <michaelni at gmx.at> writes:
> 
> > On Sun, Aug 24, 2008 at 03:33:36PM +0200, Aurelien Jacobs wrote:
> >> Reimar D?ffinger wrote:
> >> 
> >> > Hello,
> >> > admittedly not tested, but seems quite obvious.
> >> 
> >> That indeed looks safe.
> >> I guess you can commit such trivial patches without asking.
> >> 
> >> > Unrelated, but wasn't there a macro to do the standard
> >> > sizeof(arr)/sizeof(*arr) to get the number of array
> >> > elements? I've seen quite a few code places that could
> >> > use it - either to de-hardcode assumptions or to de-uglify..
> >> 
> >> I proposed a patch a patch adding such a macro some times
> >> ago [1]. But Michael was not fond of it.
> >> I still think it is a good idea. Maybe it's time to push
> >> it again ?
> >> My original patch (attached for reference) added two macro.
> >> One for 1D array and one for 2D array. The one for 2D
> >> array had a quite bad name and wasn't much used.
> >> So for now I propose only adding this macro:
> >> 
> >> #define FF_ARRAY_SIZE(a)  (sizeof(a)/sizeof(*a))
> >> 
> >> Michael ? Are you still against this ? Strongly, or just a
> >> little bit ? ;-)
> >
> > I have no strong oppinion on this specific case but overall there are
> > IMHO FAR too many macros in ffmpeg already, and we should try to reduce
> > their number not increase it.
> > Thus if this is added then id like to see at least 1 other macro removed
> > and
> 
> What's suddenly so bad about macros?

to understand:
for(i=0; i < sizeof(a)/sizeof(*a); i++){ ...
you just need to know C

to understand
for(i=0; i < FF_ARRAY_SIZE(a); i++){ ...
you need to know what FF_ARRAY_SIZE does as well.

The more such helper functions and macros ffmpeg has and uses, the more
difficult it is for a person "outside" to understand and work with the code.

Thus when adding new ones we should really consider
* how easy to understand is it when some average to good C programmer for
  the first time sees it?
* Does he have to look up the definiton or is the name alone enough?
* what effect does it have on readability for experienced ffmpeg developers?
* how much more compact can the code be made by its use?

Its not really specific to macros ...
And i do not know if this specific case is good or bad ...
I simply do not find
sizeof(array) / sizeof(array[0])
particularely ugly ...

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The misfortune of the wise is better than the prosperity of the fool.
-- Epicurus
-------------- 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/20080824/8b58f040/attachment.pgp>