Opened 8 years ago
Closed 8 years ago
#12996 closed Patch - Bug Fix (fixed)
Fix compiler warnings in trunk
Reported by: | Owned by: | Karl Egly | |
---|---|---|---|
Priority: | minor | Milestone: | 29.0 |
Component: | MythTV - General | Version: | Master Head |
Severity: | medium | Keywords: | |
Cc: | Ticket locked: | no |
Description
Compiling trunk generates a number of warning messages. These patches fix the simple warnings.
Attachments (24)
Change History (38)
Changed 8 years ago by
Attachment: | 0001-Fix-compiler-Wmisleading-indentation-warning-message.patch added |
---|
Changed 8 years ago by
Attachment: | 0002-Fix-compiler-Wparentheses-error-message.patch added |
---|
Fix for missing parentheses in math calculation.
Changed 8 years ago by
Attachment: | 0003-Fix-compiler-Wreorder-error-message.patch added |
---|
Fix for class variable initialized in different order than its declared
Changed 8 years ago by
Attachment: | 0004-Fix-compiler-Wunused-variable-error-message.patch added |
---|
Remove unused variable
Changed 8 years ago by
Attachment: | 0005-Fix-compiler-Wnonnull-compare-error-message.patch added |
---|
Remove comparison that will always fail.
comment:1 Changed 8 years ago by
Milestone: | unknown → 29.0 |
---|---|
Version: | Unspecified → Master Head |
Changed 8 years ago by
Attachment: | 0006-Fix-compiler-unused-parameter-error-messages.patch added |
---|
Fix compiler 'unused parameter' error messages.
comment:2 Changed 8 years ago by
For 0006...., you are likely going to have to get upstream FFMpeg to accept the part of that patch that impacts that code. This project tends to prefer not to carry patches unless absolutely necessary to minimize future merge effort(s).
Changed 8 years ago by
Attachment: | 0007-Fix-compiler-comparison-of-unsigned-expression-0-is-.patch added |
---|
Fix 'comparison of unsigned expression' warning.
Changed 8 years ago by
Attachment: | 0008-Fix-compiler-comparison-is-always-false-due-to-limit.patch added |
---|
Fix 'comparison is always false' warning message.
Changed 8 years ago by
Attachment: | 0009-Fix-compiler-left-shift-of-negative-value-warning.patch added |
---|
Fix 'left fhift of ngative value' warning.
Changed 8 years ago by
Attachment: | 0010-Fix-compiler-enumeral-and-non-enumeral-type-in-condi.patch added |
---|
Fix 'enumeral and non-enumeral type' warning in mythmainwindow.cpp.
Changed 8 years ago by
Attachment: | 0011-Fix-compiler-enumeral-and-non-enumeral-type-in-condi.patch added |
---|
Fix compiler 'enumeral and non-enumeral' warning in recordinginfo.cpp.
comment:3 Changed 8 years ago by
Re: 0006-...., some (although not necessarily all) of the unused parameters fixes should likely be modified to follow the "Coding Standards" document which states:
Unused parameters Functions that have to satisfy a given interface definition sometimes have parameters that are not used. The name of an unused parameter should be put in a comment to document that the parameter is intentionally unused in such a way that using it anyway will not work. If the function does not have to follow such an interface definition the parameter should simply be removed.
So, for a specific example, a "func(int a, int b)" should turn into a "func(int /* a */, int /* b */)" if a and b are not used, OR the coding standard document needs to be updated.
comment:4 follow-up: 5 Changed 8 years ago by
Commenting out the variable name is a harder restriction than using the Q_UNUSED() macro. The former prevents any accidental use of the variable in the function, while the latter simply shuts up the compiler by generating a fake reference to the variable. I.E. Q_UNUSED(x) generates "(void)x". I used the macro as I thought that was "the Qt way", but I'm happy to rework the patch if that's what the devs would like.
comment:5 Changed 8 years ago by
Replying to David Hampton <mythtv@…>:
the latter simply shuts up the compiler by generating a fake reference to the variable.
David, thanks for joining in and helping with handling the heaps of compiler warnings.
The goal is to silence false positives and to fix all bugs that are pointed out by the warnings. Just shutting the compiler up may make it harder to actually identify genuine bugs.
Here's an example from audioconvert.h:
bool operator==(AudioConvert& rhs) const { return m_in == rhs.m_in && m_out == rhs.m_out; } bool operator!=(AudioConvert& rhs) const - { return m_in != m_out; } + { Q_UNUSED(rhs); return m_in != m_out; }
These are binary operators. And one of them is completely ignoring the right-hand-side symbol. That looks very much like a real bug.
A better fix would be mimicking the operator== like this (just making it up on the fly):
bool operator==(AudioConvert& rhs) const { return m_in == rhs.m_in && m_out == rhs.m_out; } bool operator!=(AudioConvert& rhs) const - { return m_in != m_out; } + { return m_in != rhs.m_in || m_out != rhs.m_out; }
Maybe add some extra parentheses so its easier for a human to read. (not everyone has the precedence order memorized)
bool operator==(AudioConvert& rhs) const - { return m_in == rhs.m_in && m_out == rhs.m_out; } + { return (m_in == rhs.m_in) && (m_out == rhs.m_out); } bool operator!=(AudioConvert& rhs) const - { return m_in != m_out; } + { return (m_in != rhs.m_in) || (m_out != rhs.m_out); }
comment:6 Changed 8 years ago by
Owner: | set to Karl Egly |
---|---|
Status: | new → assigned |
comment:7 Changed 8 years ago by
OK, ignore patch 0006. I'll take another run at the 'unused parameter' warnings.
Changed 8 years ago by
Attachment: | v2-0001-Simple-unused-parameter-fixes.patch added |
---|
Fix unused parameters in trivial functions.
Changed 8 years ago by
Attachment: | v2-0002-Unused-parameter-fixes-for-conditionally-used-var.patch added |
---|
Fix unused parameter warnings when the variable use is conditionally compiled.
Changed 8 years ago by
Attachment: | v2-0003-Fix-unused-parameter-warning-for-explicit-dummy-v.patch added |
---|
Remove variable explicitly named dummy.
Changed 8 years ago by
Attachment: | v2-0004-Fix-unused-parameter-warning-where-omission-seems.patch added |
---|
Silence unused parameter warnings where the non-use seems intentional.
Changed 8 years ago by
Attachment: | v2-0005-Use-parameters-that-were-flagged-as-unused-parame.patch added |
---|
Find way to use an 'unused parameter'.
Changed 8 years ago by
Attachment: | v2-0006-Fix-unused-parameter-warning-in-UPNP-serializers.patch added |
---|
UPNP serializer patches for intentionally unused variables.
Changed 8 years ago by
Attachment: | v2-0007-Clean-up-unused-parameter-warnings-in-mythutil.patch added |
---|
Clean up unused parameter warnings in mythutil.
Changed 8 years ago by
Attachment: | v2-0008-Adjust-GuideGrid-widget-for-the-parent-offsets.patch added |
---|
GuideGrid? widget should account for the parent offsets.
Changed 8 years ago by
Attachment: | v2-0009-Remove-unused-code-fragment.patch added |
---|
Remove unused code fragment in upnpdevice.cpp.
Changed 8 years ago by
Attachment: | v2-0010-Correct-the-variable-names-from-clipRegion-to-cli.patch added |
---|
Correct the variable names from clipRegion to clipRect.
Changed 8 years ago by
Attachment: | v2-0011-Add-caller-information-to-the-SetAvailableStatus-.patch added |
---|
Add caller information to the SetAvailableStatus? log message.
Changed 8 years ago by
Attachment: | v2-0012-Fix-unused-variable-warning-for-debug-code.patch added |
---|
Fix 'unused variable' warning for debug code.
Changed 8 years ago by
Attachment: | v2-0013-Remove-code-fragment-whose-result-is-never-used.patch added |
---|
Remove code fragment whose result is never used.
comment:8 Changed 8 years ago by
Submitted an updated set of patches as https://github.com/MythTV/mythtv/pull/134
comment:12 Changed 8 years ago by
David, I was cherry picking from your patchset yesterday, when your update came alogn. Can you rebase your PR?
comment:14 Changed 8 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Fix for misleading indentation.