[FFmpeg-devel] [PATCH 11/17] lavc/dcaenc: fix make checkheaders.

Michael Niedermayer michaelni at gmx.at
Wed May 9 15:33:10 CEST 2012

On Wed, May 09, 2012 at 01:25:39PM +0200, Clément Bœsch wrote:
> On Wed, May 09, 2012 at 12:45:07PM +0200, Michael Niedermayer wrote:
> > On Wed, May 09, 2012 at 10:01:36AM +0200, Clément Bœsch wrote:
> > > ---
> > >  libavcodec/dcaenc.h |    2 ++
> > >  1 file changed, 2 insertions(+)
> > 
> > does it make sense to run check headers on non public headers ?
> > 
> It's certainly simpler to check them all. Do you think all these checks
> are a problem?

I think the problem lies with how the checks are seen.
What we need to do is fix bugs, improve maintainability and readability
the checks can point to problems, these problems should be fixed.
but when the check points to correct code care must be taken not to
worsen the code for no other point than to hide the warning.
Here the check should be fixed if no trivial change to the code can
solve it.

For example i dont think moving an include from the top of the file
to the middle is really a good idea for maintainability.
Also i think os2threads should fail on non os2 systems, it could be
added to SKIPHEADERS if the OS is not OS2. adding ifdefs into it
so it can be included but does nothing at all on other platforms
doesnt seem like a good idea to me
and i did often do a grep X *.{c,h} to quickly find where something is
defined, the renamings of .h->something else would complicate this.
If these changes would be fixing bugs then sure they are ok but i see
a failure of a checkheaders script i dont see a bug where it fails


Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Asymptotically faster algorithms should always be preferred if you have
asymptotical amounts of data
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120509/e057878c/attachment.asc>

More information about the ffmpeg-devel mailing list