[systemd-devel] Removing unnecessary includes

[Lennart Poettering]

On Mon, 09.02.15 10:19, Thomas H.P. Andersen (phomes at gmail.com) wrote:

> include-what-you-use is actually pretty nice. It is also a little bit
> crazy. It wants to include everything directly and we would add a lot
> includes for errno.h, string.h, stdlib.h, stdbool.h, stddef.h, etc etc
> everywhere. It also wants to use forward declares when possible. The
> coding style does not say anything specific about this. Any
> preferences? I will use the tool to trim the unnecessary includes
> first and leave its other advise for later. It will be interesting to
> see how each step affects the build time.
> 
> A feature is that I find interesting is that it can comment each
> include with the reason that it was included. Not something to commit,
> but useful to get an overview of why each include is there.

Hmm, I find this unnecessary noise I must say...

> #include <stdbool.h>                    // for false, true, bool
> #include <stddef.h>                     // for offsetof, size_t
> #include <stdint.h>                     // for uint64_t
> #include <string.h>                     // for strdup, strlen
> #include "config.h"                     // for PACKAGE_STRING,
> #VERSION

This one we get though "-include config.h" on the gcc command line.

> #include "src/shared/macro.h"           // for assert, assert_cc, etc
> #include "src/shared/path-lookup.h"
> #include "src/shared/time-util.h"       // for format_timespan, usec_t, etc
> #include "src/systemd/sd-bus-protocol.h"  // for ::SD_BUS_TYPE_ARRAY

Nah, please no absolute includes...

Any chance this can turned off?

> The full include-list for src/analyze/analyze.c:
> #include <errno.h>                      // for EIO, ENOMEM, E2BIG, EINVAL, etc
> #include <fnmatch.h>                    // for fnmatch
> #include <getopt.h>                     // for optind, no_argument, optarg, etc
> #include <locale.h>                     // for NULL, setlocale, LC_ALL, etc
> #include <stdbool.h>                    // for false, true, bool
> #include <stddef.h>                     // for offsetof, size_t
> #include <stdint.h>                     // for uint64_t
> #include <stdio.h>                      // for printf, puts, fputs, stdout
> #include <stdlib.h>                     // for free, qsort, EXIT_FAILURE, etc
> #include <string.h>                     // for strdup, strlen
> #include "analyze-verify.h"             // for verify_units
> #include "build.h"                      // for SYSTEMD_FEATURES
> #include "bus-error.h"                  // for bus_error_message
> #include "bus-util.h"                   // for UnitInfo, etc
> #include "config.h"                     // for PACKAGE_STRING, VERSION
> #include "hashmap.h"                    // for hashmap_get, Hashmap, etc
> #include "log.h"                        // for log_error, log_oom, etc
> #include "pager.h"                      // for pager_close, pager_open
> #include "sd-bus.h"                     // for sd_bus, SD_BUS_ERROR_NULL, etc
> #include "special.h"                    // for SPECIAL_DEFAULT_TARGET
> #include "src/shared/macro.h"           // for assert, assert_cc, etc
> #include "src/shared/path-lookup.h"
> #include "src/shared/time-util.h"       // for format_timespan, usec_t, etc
> #include "src/systemd/sd-bus-protocol.h"  // for ::SD_BUS_TYPE_ARRAY
> #include "strv.h"                       // for strv_isempty, STRV_FOREACH, etc
> #include "strxcpyx.h"                   // for strpcpyf
> #include "unit-name.h"                  // for unit_dbus_path_from_name
> #include "util.h"                       // for isempty, streq, etc

Note that I actually try to roughly maintain an order when including
things: For the headers of other projects i try to put system headers
first, 3rd party headers second. For our own stuff I try to put
external API stuff first (i.e. sd-*.h), followed by internal API-like
stuff form src/shared, and finally the other headers that are prviate
to the specific module the code is in.

I'd really like to avoid a scheme where this is reordered
randomly...

The order kinda is relevant, since more "local", "specific"
definitions should never influence more "public", "generic"
interfaces, if you follow what I mean? A lot of header files use
#ifndef, and only conditionally define things then. They should not
get confused by our own definitions....

Lennart

-- 
Lennart Poettering, Red Hat