From 36f822c4bd077f9121757e24b6516e5c7ada63b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 16 Jul 2014 18:27:12 -0400 Subject: [PATCH] Let config_parse open file where applicable Special care is needed so that we get an error message if the file failed to parse, but not when it is missing. To avoid duplicating the same error check in every caller, add an additional 'warn' boolean to tell config_parse whether a message should be issued. This makes things both shorter and more robust wrt. to error reporting. --- src/bootchart/bootchart.c | 14 +++-------- src/core/load-dropin.c | 5 ++-- src/core/load-fragment.c | 7 +++--- src/core/main.c | 18 +++---------- src/dbus1-generator/dbus1-generator.c | 22 +++++----------- src/journal-remote/journal-remote.c | 13 +++------- src/journal-remote/journal-upload.c | 13 +++------- src/journal/coredump.c | 14 +++-------- src/journal/journald-server.c | 23 +++-------------- src/login/logind.c | 23 +++-------------- src/network/networkd-netdev.c | 6 ++--- src/network/networkd-network.c | 6 ++--- src/resolve/resolved-manager.c | 14 +++-------- src/shared/conf-parser.c | 25 +++++++++++++++---- src/shared/conf-parser.h | 1 + src/shared/install.c | 5 +++- src/shared/sleep-config.c | 16 +++--------- src/timesync/timesyncd.c | 25 +++---------------- .../tty-ask-password-agent.c | 23 +++++------------ src/udev/net/link-config.c | 7 +++--- 20 files changed, 90 insertions(+), 190 deletions(-) diff --git a/src/bootchart/bootchart.c b/src/bootchart/bootchart.c index 8e849da59c..cbfc28d2f1 100644 --- a/src/bootchart/bootchart.c +++ b/src/bootchart/bootchart.c @@ -124,17 +124,11 @@ static void parse_conf(void) { { "Bootchart", "ControlGroup", config_parse_bool, 0, &arg_show_cgroup }, { NULL, NULL, NULL, 0, NULL } }; - _cleanup_fclose_ FILE *f; - int r; - f = fopen(BOOTCHART_CONF, "re"); - if (!f) - return; - - r = config_parse(NULL, BOOTCHART_CONF, f, - NULL, config_item_table_lookup, items, true, false, NULL); - if (r < 0) - log_warning("Failed to parse configuration file: %s", strerror(-r)); + config_parse(NULL, BOOTCHART_CONF, NULL, + NULL, + config_item_table_lookup, items, + true, false, true, NULL); if (init != NULL) strscpy(arg_init_path, sizeof(arg_init_path), init); diff --git a/src/core/load-dropin.c b/src/core/load-dropin.c index 66547cf4bc..21c991526c 100644 --- a/src/core/load-dropin.c +++ b/src/core/load-dropin.c @@ -186,8 +186,9 @@ int unit_load_dropin(Unit *u) { STRV_FOREACH(f, u->dropin_paths) { config_parse(u->id, *f, NULL, - UNIT_VTABLE(u)->sections, config_item_perf_lookup, - load_fragment_gperf_lookup, false, false, u); + UNIT_VTABLE(u)->sections, + config_item_perf_lookup, load_fragment_gperf_lookup, + false, false, false, u); } u->dropin_mtime = now(CLOCK_REALTIME); diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 54010b804e..81f137946c 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -3368,9 +3368,10 @@ static int load_from_path(Unit *u, const char *path) { u->load_state = UNIT_LOADED; /* Now, parse the file contents */ - r = config_parse(u->id, filename, f, UNIT_VTABLE(u)->sections, - config_item_perf_lookup, - load_fragment_gperf_lookup, false, true, u); + r = config_parse(u->id, filename, f, + UNIT_VTABLE(u)->sections, + config_item_perf_lookup, load_fragment_gperf_lookup, + false, true, false, u); if (r < 0) return r; } diff --git a/src/core/main.c b/src/core/main.c index d1fb265df1..f9ee297afa 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -682,23 +682,13 @@ static int parse_config_file(void) { {} }; - _cleanup_fclose_ FILE *f; const char *fn; - int r; fn = arg_running_as == SYSTEMD_SYSTEM ? PKGSYSCONFDIR "/system.conf" : PKGSYSCONFDIR "/user.conf"; - f = fopen(fn, "re"); - if (!f) { - if (errno == ENOENT) - return 0; - - log_warning("Failed to open configuration file '%s': %m", fn); - return 0; - } - - r = config_parse(NULL, fn, f, "Manager\0", config_item_table_lookup, items, false, false, NULL); - if (r < 0) - log_warning("Failed to parse configuration file: %s", strerror(-r)); + config_parse(NULL, fn, NULL, + "Manager\0", + config_item_table_lookup, items, + false, false, true, NULL); return 0; } diff --git a/src/dbus1-generator/dbus1-generator.c b/src/dbus1-generator/dbus1-generator.c index ba2953014d..dcfceecf63 100644 --- a/src/dbus1-generator/dbus1-generator.c +++ b/src/dbus1-generator/dbus1-generator.c @@ -163,27 +163,17 @@ static int add_dbus(const char *path, const char *fname, const char *type) { { "D-BUS Service", "SystemdService", config_parse_string, 0, &service }, }; - _cleanup_fclose_ FILE *f = NULL; - _cleanup_free_ char *p = NULL; + char *p; int r; assert(path); assert(fname); - p = strjoin(path, "/", fname, NULL); - if (!p) - return log_oom(); - - f = fopen(p, "re"); - if (!f) { - if (errno == -ENOENT) - return 0; - - log_error("Failed to read %s: %m", p); - return -errno; - } - - r = config_parse(NULL, p, f, "D-BUS Service\0", config_item_table_lookup, table, true, false, NULL); + p = strappenda3(path, "/", fname); + r = config_parse(NULL, p, NULL, + "D-BUS Service\0", + config_item_table_lookup, table, + true, false, true, NULL); if (r < 0) return r; diff --git a/src/journal-remote/journal-remote.c b/src/journal-remote/journal-remote.c index 08de9d03ae..37ad9029fd 100644 --- a/src/journal-remote/journal-remote.c +++ b/src/journal-remote/journal-remote.c @@ -1122,16 +1122,11 @@ static int parse_config(void) { { "Remote", "ServerCertificateFile", config_parse_path, 0, &arg_cert }, { "Remote", "TrustedCertificateFile", config_parse_path, 0, &arg_trust }, {}}; - int r; - r = config_parse(NULL, PKGSYSCONFDIR "/journal-remote.conf", NULL, - "Remote\0", - config_item_table_lookup, items, - false, false, NULL); - if (r < 0) - log_error("Failed to parse configuration file: %s", strerror(-r)); - - return r; + return config_parse(NULL, PKGSYSCONFDIR "/journal-remote.conf", NULL, + "Remote\0", + config_item_table_lookup, items, + false, false, true, NULL); } static void help(void) { diff --git a/src/journal-remote/journal-upload.c b/src/journal-remote/journal-upload.c index a381ec59bf..72396bf205 100644 --- a/src/journal-remote/journal-upload.c +++ b/src/journal-remote/journal-upload.c @@ -495,16 +495,11 @@ static int parse_config(void) { { "Upload", "ServerCertificateFile", config_parse_path, 0, &arg_cert }, { "Upload", "TrustedCertificateFile", config_parse_path, 0, &arg_trust }, {}}; - int r; - r = config_parse(NULL, PKGSYSCONFDIR "/journal-upload.conf", NULL, - "Upload\0", - config_item_table_lookup, items, - false, false, NULL); - if (r < 0) - log_error("Failed to parse configuration file: %s", strerror(-r)); - - return r; + return config_parse(NULL, PKGSYSCONFDIR "/journal-upload.conf", NULL, + "Upload\0", + config_item_table_lookup, items, + false, false, true, NULL); } static void help(void) { diff --git a/src/journal/coredump.c b/src/journal/coredump.c index 4ac1a41299..182c2b1bad 100644 --- a/src/journal/coredump.c +++ b/src/journal/coredump.c @@ -114,16 +114,10 @@ static int parse_config(void) { {} }; - return config_parse( - NULL, - "/etc/systemd/coredump.conf", - NULL, - "Coredump\0", - config_item_table_lookup, - items, - false, - false, - NULL); + return config_parse(NULL, "/etc/systemd/coredump.conf", NULL, + "Coredump\0", + config_item_table_lookup, items, + false, false, true, NULL); } static int fix_acl(int fd, uid_t uid) { diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c index 097af24c5b..4ea9d43c0d 100644 --- a/src/journal/journald-server.c +++ b/src/journal/journald-server.c @@ -1331,27 +1331,12 @@ static int server_parse_proc_cmdline(Server *s) { } static int server_parse_config_file(Server *s) { - static const char fn[] = "/etc/systemd/journald.conf"; - _cleanup_fclose_ FILE *f = NULL; - int r; - assert(s); - f = fopen(fn, "re"); - if (!f) { - if (errno == ENOENT) - return 0; - - log_warning("Failed to open configuration file %s: %m", fn); - return -errno; - } - - r = config_parse(NULL, fn, f, "Journal\0", config_item_perf_lookup, - journald_gperf_lookup, false, false, s); - if (r < 0) - log_warning("Failed to parse configuration file: %s", strerror(-r)); - - return r; + return config_parse(NULL, "/etc/systemd/journald.conf", NULL, + "Journal\0", + config_item_perf_lookup, journald_gperf_lookup, + false, false, true, s); } static int server_dispatch_sync(sd_event_source *es, usec_t t, void *userdata) { diff --git a/src/login/logind.c b/src/login/logind.c index a1881840e1..ec5529db1a 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -1097,27 +1097,12 @@ int manager_run(Manager *m) { } static int manager_parse_config_file(Manager *m) { - static const char fn[] = "/etc/systemd/logind.conf"; - _cleanup_fclose_ FILE *f = NULL; - int r; - assert(m); - f = fopen(fn, "re"); - if (!f) { - if (errno == ENOENT) - return 0; - - log_warning("Failed to open configuration file %s: %m", fn); - return -errno; - } - - r = config_parse(NULL, fn, f, "Login\0", config_item_perf_lookup, - logind_gperf_lookup, false, false, m); - if (r < 0) - log_warning("Failed to parse configuration file: %s", strerror(-r)); - - return r; + return config_parse(NULL, "/etc/systemd/logind.conf", NULL, + "Login\0", + config_item_perf_lookup, logind_gperf_lookup, + false, false, true, m); } int main(int argc, char *argv[]) { diff --git a/src/network/networkd-netdev.c b/src/network/networkd-netdev.c index 7d12af3938..9974913f49 100644 --- a/src/network/networkd-netdev.c +++ b/src/network/networkd-netdev.c @@ -526,11 +526,9 @@ static int netdev_load_one(Manager *manager, const char *filename) { r = config_parse(NULL, filename, file, "Match\0NetDev\0VLAN\0MACVLAN\0VXLAN\0Tunnel\0Peer\0Tun\0Tap\0Bond\0", config_item_perf_lookup, network_netdev_gperf_lookup, - false, false, netdev); - if (r < 0) { - log_warning("Could not parse config file %s: %s", filename, strerror(-r)); + false, false, true, netdev); + if (r < 0) return r; - } /* skip out early if configuration does not match the environment */ if (net_match_config(NULL, NULL, NULL, NULL, NULL, diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c index 252c9a0ab0..700577fccf 100644 --- a/src/network/networkd-network.c +++ b/src/network/networkd-network.c @@ -96,11 +96,9 @@ static int network_load_one(Manager *manager, const char *filename) { r = config_parse(NULL, filename, file, "Match\0Network\0Address\0Route\0DHCP\0DHCPv4\0", config_item_perf_lookup, network_network_gperf_lookup, - false, false, network); - if (r < 0) { - log_warning("Could not parse config file %s: %s", filename, strerror(-r)); + false, false, true, network); + if (r < 0) return r; - } LIST_PREPEND(networks, manager->networks, network); diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c index 3d2979dbbe..ab504d0a75 100644 --- a/src/resolve/resolved-manager.c +++ b/src/resolve/resolved-manager.c @@ -373,18 +373,12 @@ int config_parse_dnsv( } int manager_parse_config_file(Manager *m) { - int r; - assert(m); - r = config_parse(NULL, "/etc/systemd/resolved.conf", NULL, - "Resolve\0", - config_item_perf_lookup, resolved_gperf_lookup, - false, false, m); - if (r < 0) - log_warning("Failed to parse configuration file: %s", strerror(-r)); - - return 0; + return config_parse(NULL, "/etc/systemd/resolved.conf", NULL, + "Resolve\0", + config_item_perf_lookup, resolved_gperf_lookup, + false, false, true, m); } int manager_new(Manager **ret) { diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c index 1f40986649..0be7226871 100644 --- a/src/shared/conf-parser.c +++ b/src/shared/conf-parser.c @@ -245,7 +245,7 @@ static int parse_line(const char* unit, if (!fn) return -ENOMEM; - return config_parse(unit, fn, NULL, sections, lookup, table, relaxed, false, userdata); + return config_parse(unit, fn, NULL, sections, lookup, table, relaxed, false, false, userdata); } if (*l == '[') { @@ -326,6 +326,7 @@ int config_parse(const char *unit, const void *table, bool relaxed, bool allow_include, + bool warn, void *userdata) { _cleanup_free_ char *section = NULL, *continuation = NULL; @@ -340,7 +341,11 @@ int config_parse(const char *unit, if (!f) { f = ours = fopen(filename, "re"); if (!f) { - log_full(errno == ENOENT ? LOG_DEBUG : LOG_ERR, "Failed to open configuration file '%s': %m", filename); + /* Only log on request, except for ENOENT, + * since we return 0 to the caller. */ + if (warn || errno == ENOENT) + log_full(errno == ENOENT ? LOG_DEBUG : LOG_ERR, + "Failed to open configuration file '%s': %m", filename); return errno == ENOENT ? 0 : -errno; } } @@ -363,8 +368,11 @@ int config_parse(const char *unit, if (continuation) { c = strappend(continuation, l); - if (!c) + if (!c) { + if (warn) + log_oom(); return -ENOMEM; + } free(continuation); continuation = NULL; @@ -386,8 +394,11 @@ int config_parse(const char *unit, continuation = c; else { continuation = strdup(l); - if (!continuation) + if (!continuation) { + if (warn) + log_oom(); return -ENOMEM; + } } continue; @@ -408,8 +419,12 @@ int config_parse(const char *unit, userdata); free(c); - if (r < 0) + if (r < 0) { + if (warn) + log_warning("Failed to parse file '%s': %s", + filename, strerror(-r)); return r; + } } return 0; diff --git a/src/shared/conf-parser.h b/src/shared/conf-parser.h index 3e2c784805..ea7b710dec 100644 --- a/src/shared/conf-parser.h +++ b/src/shared/conf-parser.h @@ -89,6 +89,7 @@ int config_parse(const char *unit, const void *table, bool relaxed, bool allow_include, + bool warn, void *userdata); /* Generic parsers */ diff --git a/src/shared/install.c b/src/shared/install.c index a080d8f328..bc2a377971 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -1076,7 +1076,10 @@ static int unit_file_load( return -ENOMEM; } - r = config_parse(NULL, path, f, NULL, config_item_table_lookup, items, true, true, info); + r = config_parse(NULL, path, f, + NULL, + config_item_table_lookup, items, + true, true, false, info); if (r < 0) return r; diff --git a/src/shared/sleep-config.c b/src/shared/sleep-config.c index 4b2b0fe100..16a488d56b 100644 --- a/src/shared/sleep-config.c +++ b/src/shared/sleep-config.c @@ -48,19 +48,9 @@ int parse_sleep_config(const char *verb, char ***_modes, char ***_states) { {} }; - int r; - _cleanup_fclose_ FILE *f; - - f = fopen(PKGSYSCONFDIR "/sleep.conf", "re"); - if (!f) - log_full(errno == ENOENT ? LOG_DEBUG: LOG_WARNING, - "Failed to open configuration file " PKGSYSCONFDIR "/sleep.conf: %m"); - else { - r = config_parse(NULL, PKGSYSCONFDIR "/sleep.conf", f, "Sleep\0", - config_item_table_lookup, items, false, false, NULL); - if (r < 0) - log_warning("Failed to parse configuration file: %s", strerror(-r)); - } + config_parse(NULL, PKGSYSCONFDIR "/sleep.conf", NULL, + "Sleep\0", + config_item_table_lookup, items, false, false, true, NULL); if (streq(verb, "suspend")) { /* empty by default */ diff --git a/src/timesync/timesyncd.c b/src/timesync/timesyncd.c index 67a434adee..e4e3aaecdf 100644 --- a/src/timesync/timesyncd.c +++ b/src/timesync/timesyncd.c @@ -1095,27 +1095,10 @@ int config_parse_servers( } static int manager_parse_config_file(Manager *m) { - static const char fn[] = "/etc/systemd/timesyncd.conf"; - _cleanup_fclose_ FILE *f = NULL; - int r; - - assert(m); - - f = fopen(fn, "re"); - if (!f) { - if (errno == ENOENT) - return 0; - - log_warning("Failed to open configuration file %s: %m", fn); - return -errno; - } - - r = config_parse(NULL, fn, f, "Time\0", config_item_perf_lookup, - timesyncd_gperf_lookup, false, false, m); - if (r < 0) - log_warning("Failed to parse configuration file: %s", strerror(-r)); - - return r; + return config_parse(NULL, "/etc/systemd/timesyncd.conf", NULL, + "Time\0", + config_item_perf_lookup, timesyncd_gperf_lookup, + false, false, true, m); } static bool network_is_online(void) { diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c b/src/tty-ask-password-agent/tty-ask-password-agent.c index 0398a9d814..5852f71173 100644 --- a/src/tty-ask-password-agent/tty-ask-password-agent.c +++ b/src/tty-ask-password-agent/tty-ask-password-agent.c @@ -258,25 +258,16 @@ static int parse_password(const char *filename, char **wall) { { NULL, NULL, NULL, 0, NULL } }; - FILE *f; int r; assert(filename); - f = fopen(filename, "re"); - if (!f) { - if (errno == ENOENT) - return 0; - - log_error("open(%s): %m", filename); - return -errno; - } - - r = config_parse(NULL, filename, f, NULL, config_item_table_lookup, items, true, false, NULL); - if (r < 0) { - log_error("Failed to parse password file %s: %s", filename, strerror(-r)); - goto finish; - } + r = config_parse(NULL, filename, NULL, + NULL, + config_item_table_lookup, items, + true, false, true, NULL); + if (r < 0) + return r; if (!socket_name) { log_error("Invalid password file %s", filename); @@ -414,8 +405,6 @@ static int parse_password(const char *filename, char **wall) { } finish: - fclose(f); - safe_close(socket_fd); free(packet); diff --git a/src/udev/net/link-config.c b/src/udev/net/link-config.c index fe916a4326..73243fa107 100644 --- a/src/udev/net/link-config.c +++ b/src/udev/net/link-config.c @@ -177,11 +177,10 @@ static int load_link(link_config_ctx *ctx, const char *filename) { r = config_parse(NULL, filename, file, "Match\0Link\0Ethernet\0", config_item_perf_lookup, link_config_gperf_lookup, - false, false, link); - if (r < 0) { - log_warning("Could not parse config file %s: %s", filename, strerror(-r)); + false, false, true, link); + if (r < 0) return r; - } else + else log_debug("Parsed configuration file %s", filename); link->filename = strdup(filename);