Bug 39759 - remove dead code that is only compiled when tests are enabled
Summary: remove dead code that is only compiled when tests are enabled
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: 1.5
Hardware: Other All
: medium normal
Assignee: Simon McVittie
QA Contact: John (J5) Palmieri
URL: http://cgit.freedesktop.org/~smcv/dbu...
Whiteboard:
Keywords: patch
Depends on:
Blocks: dbus-1.5
  Show dependency treegraph
 
Reported: 2011-08-02 05:03 UTC by Simon McVittie
Modified: 2012-02-13 10:23 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
_dbus_string_append_unichar, _dbus_string_get_unichar: remove (6.04 KB, patch)
2011-08-02 05:15 UTC, Simon McVittie
Details | Splinter Review
_dbus_string_append_double, _dbus_string_parse_double: remove (10.31 KB, patch)
2011-08-02 05:15 UTC, Simon McVittie
Details | Splinter Review
_dbus_getgid: remove, unused (1.20 KB, patch)
2011-08-02 05:15 UTC, Simon McVittie
Details | Splinter Review
Remove unused _dbus_string_append_4_aligned, _dbus_string_append_8_aligned (2.92 KB, patch)
2011-08-02 05:15 UTC, Simon McVittie
Details | Splinter Review
_dbus_header_field_to_string: remove, unused (2.24 KB, patch)
2011-08-02 05:15 UTC, Simon McVittie
Details | Splinter Review
_dbus_list_insert_before: remove, unused (2.44 KB, patch)
2011-08-02 05:15 UTC, Simon McVittie
Details | Splinter Review
_dbus_list_pop_last_link: remove, unused (2.58 KB, patch)
2011-08-02 05:16 UTC, Simon McVittie
Details | Splinter Review
DBUS_HASH_TWO_STRINGS, DBUS_HASH_POINTER: remove, unused (19.82 KB, patch)
2011-08-02 05:16 UTC, Simon McVittie
Details | Splinter Review
_dbus_connection_queue_received_message: remove, unused (2.44 KB, patch)
2011-08-02 05:16 UTC, Simon McVittie
Details | Splinter Review
[1/3] _dbus_list_pop_last_link: remove, unused (2.84 KB, patch)
2012-02-10 07:01 UTC, Simon McVittie
Details | Splinter Review
[2/3] _dbus_string_append_unichar, _dbus_string_get_unichar: remove (6.47 KB, patch)
2012-02-10 07:02 UTC, Simon McVittie
Details | Splinter Review
[3/3] _dbus_string_append_double, _dbus_string_parse_double: remove (10.95 KB, patch)
2012-02-10 07:02 UTC, Simon McVittie
Details | Splinter Review

Description Simon McVittie 2011-08-02 05:03:14 UTC
Various features of libdbus are neither externally-accessible nor used internally. At the moment, they're compiled if and only if DBUS_BUILD_TESTS is defined.

Some of these functions are obvious generalizations of things we do use (_dbus_list_pop_last_link, _dbus_list_insert_before), but most are just unnecessary.

If we find that we do need some of these things, we can get them back from git history by reverting their removal.
Comment 1 Simon McVittie 2011-08-02 05:15:29 UTC
Created attachment 49828 [details] [review]
_dbus_string_append_unichar, _dbus_string_get_unichar: remove

These are unused (except by their regression test!) and not visible to
external callers.
Comment 2 Simon McVittie 2011-08-02 05:15:34 UTC
Created attachment 49829 [details] [review]
_dbus_string_append_double, _dbus_string_parse_double: remove

They're unused, except by their own regression tests.
Comment 3 Simon McVittie 2011-08-02 05:15:39 UTC
Created attachment 49830 [details] [review]
_dbus_getgid: remove, unused
Comment 4 Simon McVittie 2011-08-02 05:15:44 UTC
Created attachment 49831 [details] [review]
Remove unused _dbus_string_append_4_aligned, _dbus_string_append_8_aligned
Comment 5 Simon McVittie 2011-08-02 05:15:48 UTC
Created attachment 49832 [details] [review]
_dbus_header_field_to_string: remove, unused
Comment 6 Simon McVittie 2011-08-02 05:15:54 UTC
Created attachment 49833 [details] [review]
_dbus_list_insert_before: remove, unused
Comment 7 Simon McVittie 2011-08-02 05:16:01 UTC
Created attachment 49834 [details] [review]
_dbus_list_pop_last_link: remove, unused
Comment 8 Simon McVittie 2011-08-02 05:16:07 UTC
Created attachment 49835 [details] [review]
DBUS_HASH_TWO_STRINGS, DBUS_HASH_POINTER: remove, unused
Comment 9 Simon McVittie 2011-08-02 05:16:14 UTC
Created attachment 49836 [details] [review]
_dbus_connection_queue_received_message: remove, unused
Comment 10 Lennart Poettering 2012-02-09 13:24:19 UTC
All look good to me.

(I have some doubts though that anybody would look for these bits in the git history though later on, but that shouldn't stop us from deleting dead code.)

(As a side note, how did you determine the dead code? Any tool you can recommend?)
Comment 11 Simon McVittie 2012-02-10 06:27:29 UTC
(In reply to comment #10)
> (As a side note, how did you determine the dead code? Any tool you can recommend?)

In this particular case I determined it by looking for things that were #ifdef DBUS_BUILD_TESTS, but were not actually tests.

In general I use `git grep` and intuition - "does this function look hopelessly obscure? is it interfering with maintenance? let's see if we can delete it".
Comment 12 Simon McVittie 2012-02-10 07:01:21 UTC
Created attachment 56869 [details] [review]
[1/3] _dbus_list_pop_last_link: remove, unused

---

With some changes to the test-case to avoid a gcc "assigned but unused" warning.
Comment 13 Simon McVittie 2012-02-10 07:02:04 UTC
Created attachment 56870 [details] [review]
[2/3] _dbus_string_append_unichar, _dbus_string_get_unichar:  remove

These are unused (except by their regression test!) and not visible to
external callers.

---

Now without unused variables in the test.
Comment 14 Simon McVittie 2012-02-10 07:02:32 UTC
Created attachment 56871 [details] [review]
[3/3] _dbus_string_append_double, _dbus_string_parse_double:  remove

They're unused, except by their own regression tests.

----

Now without unused variables.
Comment 15 Simon McVittie 2012-02-10 07:03:25 UTC
(In reply to comment #10)
> All look good to me.

Thanks, I applied the ones that didn't cause new compiler warnings when I rebased them, and fixed the three that did (see attached).
Comment 16 Lennart Poettering 2012-02-10 12:04:18 UTC
(In reply to comment #15)
> (In reply to comment #10)
> > All look good to me.
> 
> Thanks, I applied the ones that didn't cause new compiler warnings when I rebased them, and fixed the three that did (see attached).

These look good to me, still.
Comment 17 Simon McVittie 2012-02-13 10:23:47 UTC
Fixed in git for 1.5.10, thanks for reviewing


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.