From 0bf50960493787a76d542356a93bb22f22af8072 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 24 Apr 2016 11:31:19 -0400 Subject: [PATCH 1/6] machinectl: simplify option string assignment It's better to avoid having the option string duplicated, lest we forget to modify them in sync in the future. --- src/machine/machinectl.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/machine/machinectl.c b/src/machine/machinectl.c index b03198bbf1..5a68c4ceb2 100644 --- a/src/machine/machinectl.c +++ b/src/machine/machinectl.c @@ -2516,14 +2516,9 @@ static int parse_argv(int argc, char *argv[]) { assert(argv); for (;;) { - const char *option_string; + const char * const option_string = "+hp:als:H:M:qn:o:"; - if (reorder) - option_string = "hp:als:H:M:qn:o:"; - else - option_string = "+hp:als:H:M:qn:o:"; - - c = getopt_long(argc, argv, option_string, options, NULL); + c = getopt_long(argc, argv, option_string + reorder, options, NULL); if (c < 0) { /* We generally are fine with the fact that getopt_long() reorders the command line, and looks * for switches after the main verb. However, for "shell" we really don't want that, since we From b1e02ee7cf27654359f3b7843df4d4d4b52608cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 24 Apr 2016 21:20:26 -0400 Subject: [PATCH 2/6] networkd: drop unnecessary stmt --- src/network/networkd-link.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 5101d553d5..8d6b9cce3d 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -576,8 +576,6 @@ static void link_set_state(Link *link, LinkState state) { link->state = state; link_send_changed(link, "AdministrativeState", NULL); - - return; } static void link_enter_unmanaged(Link *link) { From ddb3706d2643461a9b03044bd215e53c981d755b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 27 Apr 2016 08:59:12 -0400 Subject: [PATCH 3/6] basic/dirent-util: do not call hidden_file_allow_backup from dirent_is_file_with_suffix If the file name is supposed to end in a suffix, there's not need to check the name against a list of "special" file names, which is slow. Instead, just check that the name doens't start with a period. --- src/basic/dirent-util.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/basic/dirent-util.c b/src/basic/dirent-util.c index 5fb535cb13..5019882a0a 100644 --- a/src/basic/dirent-util.c +++ b/src/basic/dirent-util.c @@ -55,9 +55,7 @@ bool dirent_is_file(const struct dirent *de) { if (hidden_file(de->d_name)) return false; - if (de->d_type != DT_REG && - de->d_type != DT_LNK && - de->d_type != DT_UNKNOWN) + if (!IN_SET(de->d_type, DT_REG, DT_LNK, DT_UNKNOWN)) return false; return true; @@ -66,12 +64,10 @@ bool dirent_is_file(const struct dirent *de) { bool dirent_is_file_with_suffix(const struct dirent *de, const char *suffix) { assert(de); - if (de->d_type != DT_REG && - de->d_type != DT_LNK && - de->d_type != DT_UNKNOWN) + if (!IN_SET(de->d_type, DT_REG, DT_LNK, DT_UNKNOWN)) return false; - if (hidden_file_allow_backup(de->d_name)) + if (de->d_name[0] == '.') return false; return endswith(de->d_name, suffix); From 55cdd057b9ab5190150b97ae26a6b7905595ba8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 27 Apr 2016 09:24:59 -0400 Subject: [PATCH 4/6] tree-wide: rename hidden_file to hidden_or_backup_file and optimize In standard linux parlance, "hidden" usually means that the file name starts with ".", and nothing else. Rename the function to convey what the function does better to casual readers. Stop exposing hidden_file_allow_backup which is rather ugly and rewrite hidden_file to extract the suffix first. Note that hidden_file_allow_backup excluded files with "~" at the end, which is quite confusing. Let's get rid of it before it gets used in the wrong place. --- src/basic/dirent-util.c | 4 +- src/basic/dirent-util.h | 2 +- src/basic/fd-util.c | 2 +- src/basic/fdset.c | 2 +- src/basic/path-util.c | 54 +++++++++---------- src/basic/path-util.h | 3 +- src/basic/util.c | 2 +- src/shared/dropin.c | 2 +- .../tty-ask-password-agent.c | 2 +- 9 files changed, 36 insertions(+), 37 deletions(-) diff --git a/src/basic/dirent-util.c b/src/basic/dirent-util.c index 5019882a0a..59067121b7 100644 --- a/src/basic/dirent-util.c +++ b/src/basic/dirent-util.c @@ -52,10 +52,10 @@ int dirent_ensure_type(DIR *d, struct dirent *de) { bool dirent_is_file(const struct dirent *de) { assert(de); - if (hidden_file(de->d_name)) + if (!IN_SET(de->d_type, DT_REG, DT_LNK, DT_UNKNOWN)) return false; - if (!IN_SET(de->d_type, DT_REG, DT_LNK, DT_UNKNOWN)) + if (hidden_or_backup_file(de->d_name)) return false; return true; diff --git a/src/basic/dirent-util.h b/src/basic/dirent-util.h index 6bf099b46c..b91d04908f 100644 --- a/src/basic/dirent-util.h +++ b/src/basic/dirent-util.h @@ -38,7 +38,7 @@ bool dirent_is_file_with_suffix(const struct dirent *de, const char *suffix) _pu on_error; \ } \ break; \ - } else if (hidden_file((de)->d_name)) \ + } else if (hidden_or_backup_file((de)->d_name)) \ continue; \ else diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c index 3d46d708c7..9130d023d7 100644 --- a/src/basic/fd-util.c +++ b/src/basic/fd-util.c @@ -231,7 +231,7 @@ int close_all_fds(const int except[], unsigned n_except) { while ((de = readdir(d))) { int fd = -1; - if (hidden_file(de->d_name)) + if (hidden_or_backup_file(de->d_name)) continue; if (safe_atoi(de->d_name, &fd) < 0) diff --git a/src/basic/fdset.c b/src/basic/fdset.c index 06f8ecbdbc..527f27bc67 100644 --- a/src/basic/fdset.c +++ b/src/basic/fdset.c @@ -151,7 +151,7 @@ int fdset_new_fill(FDSet **_s) { while ((de = readdir(d))) { int fd = -1; - if (hidden_file(de->d_name)) + if (hidden_or_backup_file(de->d_name)) continue; r = safe_atoi(de->d_name, &fd); diff --git a/src/basic/path-util.c b/src/basic/path-util.c index 25aa355397..100e3f5af2 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -756,37 +756,37 @@ char *file_in_same_dir(const char *path, const char *filename) { return ret; } -bool hidden_file_allow_backup(const char *filename) { +bool hidden_or_backup_file(const char *filename) { + const char *p; + assert(filename); - return - filename[0] == '.' || - streq(filename, "lost+found") || - streq(filename, "aquota.user") || - streq(filename, "aquota.group") || - endswith(filename, ".rpmnew") || - endswith(filename, ".rpmsave") || - endswith(filename, ".rpmorig") || - endswith(filename, ".dpkg-old") || - endswith(filename, ".dpkg-new") || - endswith(filename, ".dpkg-tmp") || - endswith(filename, ".dpkg-dist") || - endswith(filename, ".dpkg-bak") || - endswith(filename, ".dpkg-backup") || - endswith(filename, ".dpkg-remove") || - endswith(filename, ".ucf-new") || - endswith(filename, ".ucf-old") || - endswith(filename, ".ucf-dist") || - endswith(filename, ".swp"); -} - -bool hidden_file(const char *filename) { - assert(filename); - - if (endswith(filename, "~")) + if (filename[0] == '.' || + streq(filename, "lost+found") || + streq(filename, "aquota.user") || + streq(filename, "aquota.group") || + endswith(filename, "~")) return true; - return hidden_file_allow_backup(filename); + p = strrchr(filename, '.'); + if (!p) + return false; + + return STR_IN_SET(p + 1, + "rpmnew", + "rpmsave", + "rpmorig", + "dpkg-old", + "dpkg-new", + "dpkg-tmp", + "dpkg-dist", + "dpkg-bak", + "dpkg-backup", + "dpkg-remove", + "ucf-new", + "ucf-old", + "ucf-dist", + "swp"); } bool is_device_path(const char *path) { diff --git a/src/basic/path-util.h b/src/basic/path-util.h index 34d5cd1570..a27c13fcc3 100644 --- a/src/basic/path-util.h +++ b/src/basic/path-util.h @@ -122,7 +122,6 @@ bool path_is_safe(const char *p) _pure_; char *file_in_same_dir(const char *path, const char *filename); -bool hidden_file_allow_backup(const char *filename); -bool hidden_file(const char *filename) _pure_; +bool hidden_or_backup_file(const char *filename) _pure_; bool is_device_path(const char *path); diff --git a/src/basic/util.c b/src/basic/util.c index b70c50047f..756c663be4 100644 --- a/src/basic/util.c +++ b/src/basic/util.c @@ -522,7 +522,7 @@ int on_ac_power(void) { if (!de) break; - if (hidden_file(de->d_name)) + if (hidden_or_backup_file(de->d_name)) continue; device = openat(dirfd(d), de->d_name, O_DIRECTORY|O_RDONLY|O_CLOEXEC|O_NOCTTY); diff --git a/src/shared/dropin.c b/src/shared/dropin.c index cc1acd6f23..b9cd952ac8 100644 --- a/src/shared/dropin.c +++ b/src/shared/dropin.c @@ -160,7 +160,7 @@ static int iterate_dir( if (!de) break; - if (hidden_file(de->d_name)) + if (hidden_or_backup_file(de->d_name)) continue; f = strjoin(path, "/", de->d_name, NULL); 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 7b67831e54..c7ded451a2 100644 --- a/src/tty-ask-password-agent/tty-ask-password-agent.c +++ b/src/tty-ask-password-agent/tty-ask-password-agent.c @@ -481,7 +481,7 @@ static int show_passwords(void) { if (de->d_type != DT_REG) continue; - if (hidden_file(de->d_name)) + if (hidden_or_backup_file(de->d_name)) continue; if (!startswith(de->d_name, "ask.")) From b05b9cde124d11f18d84abb61a27b6d0c75d4448 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 28 Apr 2016 08:24:25 -0400 Subject: [PATCH 5/6] test-path-util: add a trivial test for hidden_or_backup_file --- src/test/test-path-util.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c index 5d77e2959c..b53324b5e6 100644 --- a/src/test/test-path-util.c +++ b/src/test/test-path-util.c @@ -489,6 +489,27 @@ static void test_filename_is_valid(void) { assert_se(filename_is_valid("o.o")); } +static void test_hidden_or_backup_file(void) { + assert_se(hidden_or_backup_file(".hidden")); + assert_se(hidden_or_backup_file("..hidden")); + assert_se(!hidden_or_backup_file("hidden.")); + + assert_se(hidden_or_backup_file("backup~")); + assert_se(hidden_or_backup_file(".backup~")); + + assert_se(hidden_or_backup_file("lost+found")); + assert_se(hidden_or_backup_file("aquota.user")); + assert_se(hidden_or_backup_file("aquota.group")); + + assert_se(hidden_or_backup_file("test.rpmnew")); + assert_se(hidden_or_backup_file("test.dpkg-old")); + assert_se(hidden_or_backup_file("test.dpkg-remove")); + assert_se(hidden_or_backup_file("test.swp")); + + assert_se(!hidden_or_backup_file("test.rpmnew.")); + assert_se(!hidden_or_backup_file("test.dpkg-old.foo")); +} + int main(int argc, char **argv) { test_path(); test_find_binary(argv[0]); @@ -502,6 +523,7 @@ int main(int argc, char **argv) { test_path_is_mount_point(); test_file_in_same_dir(); test_filename_is_valid(); + test_hidden_or_backup_file(); return 0; } From 1f7be300e9c98a5f3b473d203dadcd9abef69baa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 28 Apr 2016 08:24:53 -0400 Subject: [PATCH 6/6] test: chmod +x sysv-generator-test Just for convenience. --- test/sysv-generator-test.py | 2 ++ 1 file changed, 2 insertions(+) mode change 100644 => 100755 test/sysv-generator-test.py diff --git a/test/sysv-generator-test.py b/test/sysv-generator-test.py old mode 100644 new mode 100755 index aca5f1eec6..aadc29ebeb --- a/test/sysv-generator-test.py +++ b/test/sysv-generator-test.py @@ -1,3 +1,5 @@ +#!/usr/bin/python +# # systemd-sysv-generator integration test # # (C) 2015 Canonical Ltd.