Bug 39231 - clean up unused variables, dead code
Summary: clean up unused variables, dead code
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: 1.5
Hardware: Other All
: low trivial
Assignee: Simon McVittie
QA Contact: John (J5) Palmieri
URL: http://cgit.freedesktop.org/~smcv/dbu...
Whiteboard:
Keywords: patch
Depends on: 38005 39230
Blocks: dbus-1.5
  Show dependency treegraph
 
Reported: 2011-07-14 11:01 UTC by Simon McVittie
Modified: 2011-09-21 04:08 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
Enable more compiler warnings by default (2.50 KB, patch)
2011-08-05 06:12 UTC, Simon McVittie
Details | Splinter Review

Description Simon McVittie 2011-07-14 11:01:50 UTC
dbus can't be compiled with -Werror=unused, even in a "normal" configuration. We should fix the warnings so we can turn on -Werror=unused all the time, because unused variables can indicate genuine bugs, like Bug #39230.
Comment 1 Simon McVittie 2011-07-14 11:03:46 UTC
Here are patches. I'll spare you the attachment bugspam, because there are 11 so far.

Bug #39230 and Bug #38005 also produce warnings, which are not addressed here.

http://cgit.freedesktop.org/~smcv/dbus/log/?h=unused-39231
Comment 2 Will Thompson 2011-08-05 03:53:46 UTC
All eleven patches—up to and including “_dbus_lookup_session_address_launchd: don't define if not used”—look fine to me.
Comment 3 Simon McVittie 2011-08-05 06:08:54 UTC
Thanks, merged for 1.5.8. I'll leave this bug open, because there are more unused variables if you either:

* build on master, or
* don't assert, or
* don't return_if_fail

Branch more-unused-39231 fixes more warnings.

Eventually, I'd like to apply the patch that I'm about to attach, to make unused stuff fatal; but it's blocked by Bug #39230 as well as this one.
Comment 4 Simon McVittie 2011-08-05 06:12:28 UTC
Created attachment 49955 [details] [review]
Enable more compiler warnings by default

Blocked by this bug and Bug #39230. When applying it, I'll also have to check that no more unused things have been introduced by then.

This enables -Wextra, except for -Wunused-parameter (intentional), -Wmissing-field-initializers (intentional), -Wtype-limits (probably a bug, we can open one later), and -Wunused-label if either assertions or checks are disabled.

(Rationale for -Wunused-label: unused labels rarely cause dead code, and at least one place in libdbus jumps to a label only if return_if_fail-style checks are enabled.)
Comment 5 Will Thompson 2011-09-21 02:01:16 UTC
more-unused-39231 looks fine, as does the attached patch to enable more warnings (in principle).
Comment 6 Simon McVittie 2011-09-21 04:08:09 UTC
Fixed in git for 1.5.8. Unused variables, etc., are now considered to be an error.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.