Opened 2 years ago

Closed 19 months ago

#12996 closed Patch - Bug Fix (fixed)

Fix compiler warnings in trunk

Reported by: David Hampton <mythtv@…> 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)

0001-Fix-compiler-Wmisleading-indentation-warning-message.patch (582 bytes) - added by David Hampton <mythtv@…> 2 years ago.
Fix for misleading indentation.
0002-Fix-compiler-Wparentheses-error-message.patch (712 bytes) - added by David Hampton <mythtv@…> 2 years ago.
Fix for missing parentheses in math calculation.
0003-Fix-compiler-Wreorder-error-message.patch (809 bytes) - added by David Hampton <mythtv@…> 2 years ago.
Fix for class variable initialized in different order than its declared
0004-Fix-compiler-Wunused-variable-error-message.patch (636 bytes) - added by David Hampton <mythtv@…> 2 years ago.
Remove unused variable
0005-Fix-compiler-Wnonnull-compare-error-message.patch (603 bytes) - added by David Hampton <mythtv@…> 2 years ago.
Remove comparison that will always fail.
0006-Fix-compiler-unused-parameter-error-messages.patch (138.4 KB) - added by David Hampton <mythtv@…> 2 years ago.
Fix compiler 'unused parameter' error messages.
0007-Fix-compiler-comparison-of-unsigned-expression-0-is-.patch (869 bytes) - added by David Hampton <mythtv@…> 2 years ago.
Fix 'comparison of unsigned expression' warning.
0008-Fix-compiler-comparison-is-always-false-due-to-limit.patch (1.2 KB) - added by David Hampton <mythtv@…> 2 years ago.
Fix 'comparison is always false' warning message.
0009-Fix-compiler-left-shift-of-negative-value-warning.patch (755 bytes) - added by David Hampton <mythtv@…> 2 years ago.
Fix 'left fhift of ngative value' warning.
0010-Fix-compiler-enumeral-and-non-enumeral-type-in-condi.patch (1.7 KB) - added by David Hampton <mythtv@…> 2 years ago.
Fix 'enumeral and non-enumeral type' warning in mythmainwindow.cpp.
0011-Fix-compiler-enumeral-and-non-enumeral-type-in-condi.patch (1.7 KB) - added by David Hampton <mythtv@…> 2 years ago.
Fix compiler 'enumeral and non-enumeral' warning in recordinginfo.cpp.
v2-0001-Simple-unused-parameter-fixes.patch (55.9 KB) - added by David Hampton <mythtv@…> 2 years ago.
Fix unused parameters in trivial functions.
v2-0002-Unused-parameter-fixes-for-conditionally-used-var.patch (4.6 KB) - added by David Hampton <mythtv@…> 2 years ago.
Fix unused parameter warnings when the variable use is conditionally compiled.
v2-0003-Fix-unused-parameter-warning-for-explicit-dummy-v.patch (1016 bytes) - added by David Hampton <mythtv@…> 2 years ago.
Remove variable explicitly named dummy.
v2-0004-Fix-unused-parameter-warning-where-omission-seems.patch (18.2 KB) - added by David Hampton <mythtv@…> 2 years ago.
Silence unused parameter warnings where the non-use seems intentional.
v2-0005-Use-parameters-that-were-flagged-as-unused-parame.patch (3.8 KB) - added by David Hampton <mythtv@…> 2 years ago.
Find way to use an 'unused parameter'.
v2-0006-Fix-unused-parameter-warning-in-UPNP-serializers.patch (5.0 KB) - added by David Hampton <mythtv@…> 2 years ago.
UPNP serializer patches for intentionally unused variables.
v2-0007-Clean-up-unused-parameter-warnings-in-mythutil.patch (4.1 KB) - added by David Hampton <mythtv@…> 2 years ago.
Clean up unused parameter warnings in mythutil.
v2-0008-Adjust-GuideGrid-widget-for-the-parent-offsets.patch (13.8 KB) - added by David Hampton <mythtv@…> 2 years ago.
GuideGrid? widget should account for the parent offsets.
v2-0009-Remove-unused-code-fragment.patch (1.4 KB) - added by David Hampton <mythtv@…> 2 years ago.
Remove unused code fragment in upnpdevice.cpp.
v2-0010-Correct-the-variable-names-from-clipRegion-to-cli.patch (2.6 KB) - added by David Hampton <mythtv@…> 2 years ago.
Correct the variable names from clipRegion to clipRect.
v2-0011-Add-caller-information-to-the-SetAvailableStatus-.patch (1.1 KB) - added by David Hampton <mythtv@…> 2 years ago.
Add caller information to the SetAvailableStatus? log message.
v2-0012-Fix-unused-variable-warning-for-debug-code.patch (1.1 KB) - added by David Hampton <mythtv@…> 2 years ago.
Fix 'unused variable' warning for debug code.
v2-0013-Remove-code-fragment-whose-result-is-never-used.patch (938 bytes) - added by David Hampton <mythtv@…> 2 years ago.
Remove code fragment whose result is never used.

Download all attachments as: .zip

Change History (38)

Changed 2 years ago by David Hampton <mythtv@…>

Fix for misleading indentation.

Changed 2 years ago by David Hampton <mythtv@…>

Fix for missing parentheses in math calculation.

Changed 2 years ago by David Hampton <mythtv@…>

Fix for class variable initialized in different order than its declared

Changed 2 years ago by David Hampton <mythtv@…>

Remove unused variable

Changed 2 years ago by David Hampton <mythtv@…>

Remove comparison that will always fail.

comment:1 Changed 2 years ago by Stuart Auchterlonie

Milestone: unknown29.0
Version: UnspecifiedMaster Head

Changed 2 years ago by David Hampton <mythtv@…>

Fix compiler 'unused parameter' error messages.

comment:2 Changed 2 years ago by Gary Buhrmaster <gary.buhrmaster@…>

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 2 years ago by David Hampton <mythtv@…>

Fix 'comparison of unsigned expression' warning.

Changed 2 years ago by David Hampton <mythtv@…>

Fix 'comparison is always false' warning message.

Changed 2 years ago by David Hampton <mythtv@…>

Fix 'left fhift of ngative value' warning.

Changed 2 years ago by David Hampton <mythtv@…>

Fix 'enumeral and non-enumeral type' warning in mythmainwindow.cpp.

Changed 2 years ago by David Hampton <mythtv@…>

Fix compiler 'enumeral and non-enumeral' warning in recordinginfo.cpp.

comment:3 Changed 2 years ago by Gary Buhrmaster <gary.buhrmaster@…>

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 Changed 2 years ago by David Hampton <mythtv@…>

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 in reply to:  4 Changed 2 years ago by Karl Egly

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 2 years ago by Stuart Auchterlonie

Owner: set to Karl Egly
Status: newassigned

comment:7 Changed 2 years ago by David Hampton <mythtv@…>

OK, ignore patch 0006. I'll take another run at the 'unused parameter' warnings.

Changed 2 years ago by David Hampton <mythtv@…>

Fix unused parameters in trivial functions.

Changed 2 years ago by David Hampton <mythtv@…>

Fix unused parameter warnings when the variable use is conditionally compiled.

Changed 2 years ago by David Hampton <mythtv@…>

Remove variable explicitly named dummy.

Changed 2 years ago by David Hampton <mythtv@…>

Silence unused parameter warnings where the non-use seems intentional.

Changed 2 years ago by David Hampton <mythtv@…>

Find way to use an 'unused parameter'.

Changed 2 years ago by David Hampton <mythtv@…>

UPNP serializer patches for intentionally unused variables.

Changed 2 years ago by David Hampton <mythtv@…>

Clean up unused parameter warnings in mythutil.

Changed 2 years ago by David Hampton <mythtv@…>

GuideGrid? widget should account for the parent offsets.

Changed 2 years ago by David Hampton <mythtv@…>

Remove unused code fragment in upnpdevice.cpp.

Changed 2 years ago by David Hampton <mythtv@…>

Correct the variable names from clipRegion to clipRect.

Changed 2 years ago by David Hampton <mythtv@…>

Add caller information to the SetAvailableStatus? log message.

Changed 2 years ago by David Hampton <mythtv@…>

Fix 'unused variable' warning for debug code.

Changed 2 years ago by David Hampton <mythtv@…>

Remove code fragment whose result is never used.

comment:8 Changed 2 years ago by David Hampton <mythtv@…>

Submitted an updated set of patches as https://github.com/MythTV/mythtv/pull/134

comment:9 Changed 2 years ago by Karl Dietz <dekarl@…>

In 903363043f66c8271cd62d5c9a6bded5a3916fea/mythtv:

Simple unused parameter fixes.

Cherry-picked from a patch by David Hampton

Refs #12996

comment:10 Changed 2 years ago by Karl Dietz <dekarl@…>

In 3f20bc5e3152563ba72a57776022aad75b60a3f5/mythtv:

Use parameters that were flagged as 'unused parameter' warnings.

Cherry-picked from a patch by David Hampton

Refs #12996

comment:11 Changed 2 years ago by Karl Dietz <dekarl@…>

In b1f4d218a29b2db7735597359997ed6a2a29cbb3/mythtv:

Fix unused parameter warning in UPNP serializers.

Patch by David Hampton

Refs #12996

comment:12 Changed 2 years ago by Karl Egly

David, I was cherry picking from your patchset yesterday, when your update came alogn. Can you rebase your PR?

comment:13 Changed 2 years ago by David Hampton <mythtv@…>

Pull request rebased off 79844ba.

comment:14 Changed 19 months ago by David Hampton <mythtv@…>

Resolution: fixed
Status: assignedclosed

In 9506b88b3bc2c3717b32102ece45794fa71f791b/mythtv:

Cleanup gcc and clang warnings generated by -Wextra switch.

This commit fixes a large majority of the warnings generated by using
the -Wextra switch to gcc and clang. It does not fix any warning
related to objects that use QObject as their base class. Enabling the
-Wextra flag by default will occur in a later commit.

Trac: fixes #12996
Github: fixes #134

Note: See TracTickets for help on using tickets.