[mythtv] 0.27 code cleanliness

Gary Buhrmaster gary.buhrmaster at gmail.com
Tue Oct 9 01:45:55 UTC 2012


Devs,

It is my understand that one of the goals for 0.27 is general
code "cleanliness", with the result that there are fewer warnings
(with -Wall) so that any new ones might suggest something to
look further at (right now the posture child is all the overloaded
virtual function hidden warnings).

I know that clang is not an officially supported compiler, but
when compiling mythtv under clang, a number of warnings are
produced of the form:


  RTjpegN.cpp:3098:22: warning: use of GNU old-style field designator
extension [-Wgnu-designator]
   static mmx_t neg= { uq: (unsigned long long)0xffffffffffffffffULL };
                       ^~~
                       .uq =

As part of general code cleanup, I can contribute patches to
replace the var: syntax with .var syntax.  If I do so, is it likely
that such patches will be accepted.

Note that the newer style designator have been supported in
gcc for a long long time.

I will note that most of these warnings are in various filters
imported from other software.  If someone is planning to
rework the filters, than any work would be for naught.


Another construct that clang finds "suspicious" and produces
a warning for things like "if ((var == value))":
  avformatdecoder.cpp:187:22: warning: equality comparison with
extraneous parentheses [-Wparentheses-equality]
    if ((verboseMask == 0))
         ~~~~~~~~~~~~^~~~
  avformatdecoder.cpp:187:22: note: remove extraneous parentheses
around the comparison to silence this warning
    if ((verboseMask == 0))
          ~            ^   ~
  avformatdecoder.cpp:187:22: note: use '=' to turn this equality
comparison into an assignment
    if ((verboseMask == 0))
                       ^~
                       =

Same questions (if I contribute patches, are they likely to
be accepted).  In all the cases I have scanned, the intent
was to compare (and not assign), so the "patch" would be
to eliminate the extra parens.


And then there is the construct that clang finds upsetting
in the qt_check_for_QOBJECT_macro, which is documented at
http://qt.gitorious.org/qt/qtbase/commit/1416f4c595d6078c08f93483695e0b64c7fbb2a7/
and I guess I will just have to live with until it is accepted
by the Qt team (and use -Wno-error=self-assign until then).


Thanks for any feedback.

Gary


More information about the mythtv-dev mailing list