docs/DESKTOP_ENVIRONMENTS.md

@@ -97,13 +97,13 @@ to create appropriate units for the autostart directory
 (`systemd-xdg-autostart-generator`).
 Desktop Environments can opt-in to using this by starting
 `xdg-desktop-autostart.target`. The systemd generator correctly handles
-`OnlyShowIn=` and `NotShowIn=`. It also handles the KDE and GNOME specific
+`OnlyShowIn=` and `NotShowin=`. It also handles the KDE and GNOME specific
-`X-KDE-autostart-condition=` and `AutostartCondition=` by using desktop-environment-provided
+`X-KDE-autostart-condition=` and `AutostartCondition=` by using desktop
-binaries in an `ExecCondition=` line.
+environment provided binaries in an `ExecCondition=` line.
 
 However, this generator is somewhat limited in what it supports. For example,
 all generated units will have `After=graphical-session.target` set on them,
-and therefore may not be useful to start session services.
+it may therefore not be useful to start session services.
 
 Desktop files can be marked to be explicitly excluded from the generator using the line
 `X-systemd-skip=true`. This should be set if an application provides its own

src/shared/rm-rf.c

@@ -52,6 +52,7 @@ static int patch_dirfd_mode(
 }
 
 int unlinkat_harder(int dfd, const char *filename, int unlink_flags, RemoveFlags remove_flags) {
+
         mode_t old_mode;
         int r;
 
@@ -115,16 +116,15 @@ int fstatat_harder(int dfd,
         return 0;
 }
 
-static int rm_rf_inner_child(
+static int rm_rf_children_inner(
                 int fd,
                 const char *fname,
                 int is_dir,
                 RemoveFlags flags,
-                const struct stat *root_dev,
+                const struct stat *root_dev) {
-                bool allow_recursion) {
 
         struct stat st;
-        int r, q = 0;
+        int r;
 
         assert(fd >= 0);
         assert(fname);
@@ -141,7 +141,10 @@ static int rm_rf_inner_child(
         }
 
         if (is_dir) {
-                /* If root_dev is set, remove subdirectories only if device is same */
+                _cleanup_close_ int subdir_fd = -1;
+                int q;
+
+                /* if root_dev is set, remove subdirectories only if device is same */
                 if (root_dev && st.st_dev != root_dev->st_dev)
                         return 0;
 
@@ -153,6 +156,7 @@ static int rm_rf_inner_child(
                         return 0;
 
                 if ((flags & REMOVE_SUBVOLUME) && btrfs_might_be_subvol(&st)) {
+
                         /* This could be a subvolume, try to remove it */
 
                         r = btrfs_subvol_remove_fd(fd, fname, BTRFS_REMOVE_RECURSIVE|BTRFS_REMOVE_QUOTA);
@@ -166,40 +170,31 @@ static int rm_rf_inner_child(
                                 return 1;
                 }
 
-                if (!allow_recursion)
+                subdir_fd = openat(fd, fname, O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC|O_NOFOLLOW|O_NOATIME);
-                        return -EISDIR;
-
-                int subdir_fd = openat(fd, fname, O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC|O_NOFOLLOW|O_NOATIME);
                 if (subdir_fd < 0)
                         return -errno;
 
                 /* We pass REMOVE_PHYSICAL here, to avoid doing the fstatfs() to check the file system type
                  * again for each directory */
-                q = rm_rf_children(subdir_fd, flags | REMOVE_PHYSICAL, root_dev);
+                q = rm_rf_children(TAKE_FD(subdir_fd), flags | REMOVE_PHYSICAL, root_dev);
 
-        } else if (flags & REMOVE_ONLY_DIRECTORIES)
+                r = unlinkat_harder(fd, fname, AT_REMOVEDIR, flags);
-                return 0;
-
-        r = unlinkat_harder(fd, fname, is_dir ? AT_REMOVEDIR : 0, flags);
                 if (r < 0)
                         return r;
                 if (q < 0)
                         return q;
+
+                return 1;
+
+        } else if (!(flags & REMOVE_ONLY_DIRECTORIES)) {
+                r = unlinkat_harder(fd, fname, 0, flags);
+                if (r < 0)
+                        return r;
+
                 return 1;
         }
 
-typedef struct TodoEntry {
+        return 0;
-        DIR *dir;         /* A directory that we were operating on. */
-        char *dirname;    /* The filename of that directory itself. */
-} TodoEntry;
-
-static void free_todo_entries(TodoEntry **todos) {
-        for (TodoEntry *x = *todos; x && x->dir; x++) {
-                closedir(x->dir);
-                free(x->dirname);
-        }
-
-        freep(todos);
 }
 
 int rm_rf_children(
@@ -207,56 +202,30 @@ int rm_rf_children(
                 RemoveFlags flags,
                 const struct stat *root_dev) {
 
-        _cleanup_(free_todo_entries) TodoEntry *todos = NULL;
+        _cleanup_closedir_ DIR *d = NULL;
-        size_t n_todo = 0;
-        _cleanup_free_ char *dirname = NULL; /* Set when we are recursing and want to delete ourselves */
         int ret = 0, r;
 
-        /* Return the first error we run into, but nevertheless try to go on.
-         * The passed fd is closed in all cases, including on failure. */
-
-        for (;;) {  /* This loop corresponds to the directory nesting level. */
-                _cleanup_closedir_ DIR *d = NULL;
-
-                if (n_todo > 0) {
-                        /* We know that we are in recursion here, because n_todo is set.
-                         * We need to remove the inner directory we were operating on. */
-                        assert(dirname);
-                        r = unlinkat_harder(dirfd(todos[n_todo-1].dir), dirname, AT_REMOVEDIR, flags);
-                        if (r < 0 && r != -ENOENT && ret == 0)
-                                ret = r;
-                        dirname = mfree(dirname);
-
-                        /* And now let's back out one level up */
-                        n_todo --;
-                        d = TAKE_PTR(todos[n_todo].dir);
-                        dirname = TAKE_PTR(todos[n_todo].dirname);
-
-                        assert(d);
-                        fd = dirfd(d); /* Retrieve the file descriptor from the DIR object */
-                        assert(fd >= 0);
-                } else {
-        next_fd:
         assert(fd >= 0);
+
+        /* This returns the first error we run into, but nevertheless tries to go on. This closes the passed
+         * fd, in all cases, including on failure. */
+
         d = fdopendir(fd);
         if (!d) {
                 safe_close(fd);
                 return -errno;
         }
-                        fd = dirfd(d); /* We donated the fd to fdopendir(). Let's make sure we sure we have
-                                        * the right descriptor even if it were to internally invalidate the
-                                        * one we passed. */
 
         if (!(flags & REMOVE_PHYSICAL)) {
                 struct statfs sfs;
 
-                                if (fstatfs(fd, &sfs) < 0)
+                if (fstatfs(dirfd(d), &sfs) < 0)
                         return -errno;
 
                 if (is_physical_fs(&sfs)) {
-                                        /* We refuse to clean physical file systems with this call, unless
+                        /* We refuse to clean physical file systems with this call, unless explicitly
-                                         * explicitly requested. This is extra paranoia just to be sure we
+                         * requested. This is extra paranoia just to be sure we never ever remove non-state
-                                         * never ever remove non-state data. */
+                         * data. */
 
                         _cleanup_free_ char *path = NULL;
 
@@ -266,7 +235,6 @@ int rm_rf_children(
                                                strna(path));
                 }
         }
-                }
 
         FOREACH_DIRENT_ALL(de, d, return -errno) {
                 int is_dir;
@@ -274,47 +242,23 @@ int rm_rf_children(
                 if (dot_or_dot_dot(de->d_name))
                         continue;
 
-                        is_dir = de->d_type == DT_UNKNOWN ? -1 : de->d_type == DT_DIR;
+                is_dir =
+                        de->d_type == DT_UNKNOWN ? -1 :
+                        de->d_type == DT_DIR;
 
-                        r = rm_rf_inner_child(fd, de->d_name, is_dir, flags, root_dev, false);
+                r = rm_rf_children_inner(dirfd(d), de->d_name, is_dir, flags, root_dev);
-                        if (r == -EISDIR) {
+                if (r < 0 && r != -ENOENT && ret == 0)
-                                /* Push the current working state onto the todo list */
-
-                                 if (!GREEDY_REALLOC0(todos, n_todo + 2))
-                                         return log_oom();
-
-                                 _cleanup_free_ char *newdirname = strdup(de->d_name);
-                                 if (!newdirname)
-                                         return log_oom();
-
-                                 int newfd = openat(fd, de->d_name,
-                                                    O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC|O_NOFOLLOW|O_NOATIME);
-                                 if (newfd >= 0) {
-                                         todos[n_todo++] = (TodoEntry) { TAKE_PTR(d), TAKE_PTR(dirname) };
-                                         fd = newfd;
-                                         dirname = TAKE_PTR(newdirname);
-
-                                         goto next_fd;
-
-                                 } else if (errno != -ENOENT && ret == 0)
-                                         ret = -errno;
-
-                        } else if (r < 0 && r != -ENOENT && ret == 0)
                         ret = r;
         }
 
-                if (FLAGS_SET(flags, REMOVE_SYNCFS) && syncfs(fd) < 0 && ret >= 0)
+        if (FLAGS_SET(flags, REMOVE_SYNCFS) && syncfs(dirfd(d)) < 0 && ret >= 0)
                 ret = -errno;
 
-                if (n_todo == 0)
-                        break;
-        }
-
         return ret;
 }
 
 int rm_rf(const char *path, RemoveFlags flags) {
-        int fd, r, q = 0;
+        int fd, r;
 
         assert(path);
 
@@ -346,22 +290,18 @@ int rm_rf(const char *path, RemoveFlags flags) {
         }
 
         fd = open(path, O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC|O_NOFOLLOW|O_NOATIME);
-        if (fd >= 0) {
+        if (fd < 0) {
-                /* We have a dir */
-                r = rm_rf_children(fd, flags, NULL);
-
-                if (FLAGS_SET(flags, REMOVE_ROOT))
-                        q = RET_NERRNO(rmdir(path));
-        } else {
                 if (FLAGS_SET(flags, REMOVE_MISSING_OK) && errno == ENOENT)
                         return 0;
 
                 if (!IN_SET(errno, ENOTDIR, ELOOP))
                         return -errno;
 
-                if (FLAGS_SET(flags, REMOVE_ONLY_DIRECTORIES) || !FLAGS_SET(flags, REMOVE_ROOT))
+                if (FLAGS_SET(flags, REMOVE_ONLY_DIRECTORIES))
                         return 0;
 
+                if (FLAGS_SET(flags, REMOVE_ROOT)) {
+
                         if (!FLAGS_SET(flags, REMOVE_PHYSICAL)) {
                                 struct statfs s;
 
@@ -373,17 +313,28 @@ int rm_rf(const char *path, RemoveFlags flags) {
                                                                path);
                         }
 
-                r = 0;
+                        if (unlink(path) < 0) {
-                q = RET_NERRNO(unlink(path));
+                                if (FLAGS_SET(flags, REMOVE_MISSING_OK) && errno == ENOENT)
+                                        return 0;
+
+                                return -errno;
+                        }
                 }
 
-        if (r < 0)
-                return r;
-        if (q < 0 && (q != -ENOENT || !FLAGS_SET(flags, REMOVE_MISSING_OK)))
-                return q;
                 return 0;
         }
 
+        r = rm_rf_children(fd, flags, NULL);
+
+        if (FLAGS_SET(flags, REMOVE_ROOT) &&
+            rmdir(path) < 0 &&
+            r >= 0 &&
+            (!FLAGS_SET(flags, REMOVE_MISSING_OK) || errno != ENOENT))
+                r = -errno;
+
+        return r;
+}
+
 int rm_rf_child(int fd, const char *name, RemoveFlags flags) {
 
         /* Removes one specific child of the specified directory */
@@ -400,5 +351,5 @@ int rm_rf_child(int fd, const char *name, RemoveFlags flags) {
         if (FLAGS_SET(flags, REMOVE_ONLY_DIRECTORIES|REMOVE_SUBVOLUME))
                 return -EINVAL;
 
-        return rm_rf_inner_child(fd, name, -1, flags, NULL, true);
+        return rm_rf_children_inner(fd, name, -1, flags, NULL);
 }

src/systemctl/systemctl-logind.c

@@ -297,6 +297,7 @@ int logind_schedule_shutdown(void) {
 #if ENABLE_LOGIND
         _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
         const char *action;
+        const char *log_action;
         sd_bus *bus;
         int r;
 
@@ -307,19 +308,24 @@ int logind_schedule_shutdown(void) {
         switch (arg_action) {
         case ACTION_HALT:
                 action = "halt";
+                log_action = "Shutdown";
                 break;
         case ACTION_POWEROFF:
                 action = "poweroff";
+                log_action = "Shutdown";
                 break;
         case ACTION_KEXEC:
                 action = "kexec";
+                log_action = "Reboot via kexec";
                 break;
         case ACTION_EXIT:
                 action = "exit";
+                log_action = "Shutdown";
                 break;
         case ACTION_REBOOT:
         default:
                 action = "reboot";
+                log_action = "Reboot";
                 break;
         }
 
@@ -333,8 +339,9 @@ int logind_schedule_shutdown(void) {
                 return log_warning_errno(r, "Failed to call ScheduleShutdown in logind, proceeding with immediate shutdown: %s", bus_error_message(&error, r));
 
         if (!arg_quiet)
-                logind_show_shutdown();
+                log_info("%s scheduled for %s, use 'shutdown -c' to cancel.",
-
+                         log_action,
+                         FORMAT_TIMESTAMP_STYLE(arg_when, arg_timestamp_style));
         return 0;
 #else
         return log_error_errno(SYNTHETIC_ERRNO(ENOSYS),
@@ -389,16 +396,9 @@ int logind_show_shutdown(void) {
         if (isempty(action))
                 return log_error_errno(SYNTHETIC_ERRNO(ENODATA), "No scheduled shutdown.");
 
-        if (streq(action, "halt") || streq(action, "poweroff") || streq(action, "exit"))
-                action = "Shutdown";
-        else if (streq(action, "kexec"))
-                action = "Reboot via kexec";
-        else if (streq(action, "reboot"))
-                action = "Reboot";
-
         log_info("%s scheduled for %s, use 'shutdown -c' to cancel.",
                  action,
-                 FORMAT_TIMESTAMP_STYLE(elapse, arg_timestamp_style));
+                 FORMAT_TIMESTAMP_STYLE(arg_when, arg_timestamp_style));
 
         return 0;
 #else

src/xdg-autostart-generator/xdg-autostart-generator.c

@@ -44,42 +44,38 @@ static int enumerate_xdg_autostart(Hashmap *all_services) {
         STRV_FOREACH(path, autostart_dirs) {
                 _cleanup_closedir_ DIR *d = NULL;
 
-                log_debug("Scanning autostart directory \"%s\"…", *path);
                 d = opendir(*path);
                 if (!d) {
-                        log_full_errno(errno == ENOENT ? LOG_DEBUG : LOG_WARNING, errno,
+                        if (errno != ENOENT)
-                                       "Opening %s failed, ignoring: %m", *path);
+                                log_warning_errno(errno, "Opening %s failed, ignoring: %m", *path);
                         continue;
                 }
 
                 FOREACH_DIRENT(de, d, log_warning_errno(errno, "Failed to enumerate directory %s, ignoring: %m", *path)) {
+                        _cleanup_free_ char *fpath = NULL, *name = NULL;
+                        _cleanup_(xdg_autostart_service_freep) XdgAutostartService *service = NULL;
                         struct stat st;
+
                         if (fstatat(dirfd(d), de->d_name, &st, 0) < 0) {
-                                log_warning_errno(errno, "%s/%s: stat() failed, ignoring: %m", *path, de->d_name);
+                                log_warning_errno(errno, "stat() failed on %s/%s, ignoring: %m", *path, de->d_name);
                                 continue;
                         }
 
-                        if (!S_ISREG(st.st_mode)) {
+                        if (!S_ISREG(st.st_mode))
-                                log_debug("%s/%s: not a regular file, ignoring.", *path, de->d_name);
                                 continue;
-                        }
 
-                        _cleanup_free_ char *name = xdg_autostart_service_translate_name(de->d_name);
+                        name = xdg_autostart_service_translate_name(de->d_name);
                         if (!name)
                                 return log_oom();
 
-                        if (hashmap_contains(all_services, name)) {
+                        if (hashmap_contains(all_services, name))
-                                log_debug("%s/%s: we have already seen \"%s\", ignoring.",
-                                          *path, de->d_name, name);
                                 continue;
-                        }
 
-                        _cleanup_free_ char *fpath = path_join(*path, de->d_name);
+                        fpath = path_join(*path, de->d_name);
                         if (!fpath)
                                 return log_oom();
 
-                        _cleanup_(xdg_autostart_service_freep) XdgAutostartService *service =
+                        service = xdg_autostart_service_parse_desktop(fpath);
-                                xdg_autostart_service_parse_desktop(fpath);
                         if (!service)
                                 return log_oom();
                         service->name = TAKE_PTR(name);

src/xdg-autostart-generator/xdg-autostart-service.c

@@ -470,7 +470,6 @@ int xdg_autostart_format_exec_start(
 }
 
 static int xdg_autostart_generate_desktop_condition(
-                const XdgAutostartService *service,
                 FILE *f,
                 const char *test_binary,
                 const char *condition) {
@@ -484,8 +483,7 @@ static int xdg_autostart_generate_desktop_condition(
                 r = find_executable(test_binary, &gnome_autostart_condition_path);
                 if (r < 0) {
                         log_full_errno(r == -ENOENT ? LOG_DEBUG : LOG_WARNING, r,
-                                       "%s: ExecCondition executable %s not found, unit will not be started automatically: %m",
+                                       "%s not found: %m", test_binary);
-                                       service->path, test_binary);
                         fprintf(f, "# ExecCondition using %s skipped due to missing binary.\n", test_binary);
                         return r;
                 }
@@ -494,9 +492,6 @@ static int xdg_autostart_generate_desktop_condition(
                 if (!e_autostart_condition)
                         return log_oom();
 
-                log_debug("%s: ExecCondition converted to %s --condition \"%s\"…",
-                          service->path, gnome_autostart_condition_path, e_autostart_condition);
-
                 fprintf(f,
                          "ExecCondition=%s --condition \"%s\"\n",
                          gnome_autostart_condition_path,
@@ -507,7 +502,7 @@ static int xdg_autostart_generate_desktop_condition(
 }
 
 int xdg_autostart_service_generate_unit(
-                const XdgAutostartService *service,
+                XdgAutostartService *service,
                 const char *dest) {
 
         _cleanup_free_ char *path_escaped = NULL, *exec_start = NULL, *unit = NULL;
@@ -518,23 +513,23 @@ int xdg_autostart_service_generate_unit(
 
         /* Nothing to do for hidden services. */
         if (service->hidden) {
-                log_debug("%s: not generating unit, entry is hidden.", service->path);
+                log_debug("Not generating service for XDG autostart %s, it is hidden.", service->name);
                 return 0;
         }
 
         if (service->systemd_skip) {
-                log_debug("%s: not generating unit, marked as skipped by generator.", service->path);
+                log_debug("Not generating service for XDG autostart %s, should be skipped by generator.", service->name);
                 return 0;
         }
 
         /* Nothing to do if type is not Application. */
         if (!streq_ptr(service->type, "Application")) {
-                log_debug("%s: not generating unit, Type=%s is not supported.", service->path, service->type);
+                log_debug("Not generating service for XDG autostart %s, only Type=Application is supported.", service->name);
                 return 0;
         }
 
         if (!service->exec_string) {
-                log_warning("%s: not generating unit, no Exec= line.", service->path);
+                log_warning("Not generating service for XDG autostart %s, it is has no Exec= line.", service->name);
                 return 0;
         }
 
@@ -544,21 +539,24 @@ int xdg_autostart_service_generate_unit(
                 r = find_executable(service->try_exec, NULL);
                 if (r < 0) {
                         log_full_errno(r == -ENOENT ? LOG_DEBUG : LOG_WARNING, r,
-                                       "%s: not generating unit, could not find TryExec= binary %s: %m",
+                                       "Not generating service for XDG autostart %s, could not find TryExec= binary %s: %m",
-                                       service->path, service->try_exec);
+                                       service->name, service->try_exec);
                         return 0;
                 }
         }
 
         r = xdg_autostart_format_exec_start(service->exec_string, &exec_start);
         if (r < 0) {
-                log_warning_errno(r, "%s: not generating unit, error parsing Exec= line: %m", service->path);
+                log_warning_errno(r,
+                                  "Not generating service for XDG autostart %s, error parsing Exec= line: %m",
+                                  service->name);
                 return 0;
         }
 
         if (service->gnome_autostart_phase) {
                 /* There is no explicit value for the "Application" phase. */
-                log_debug("%s: not generating unit, startup phases are not supported.", service->path);
+                log_debug("Not generating service for XDG autostart %s, startup phases are not supported.",
+                          service->name);
                 return 0;
         }
 
@@ -572,7 +570,7 @@ int xdg_autostart_service_generate_unit(
 
         f = fopen(unit, "wxe");
         if (!f)
-                return log_error_errno(errno, "%s: failed to create unit file %s: %m", service->path, unit);
+                return log_error_errno(errno, "Failed to create unit file %s: %m", unit);
 
         fprintf(f,
                 "# Automatically generated by systemd-xdg-autostart-generator\n\n"
@@ -637,18 +635,19 @@ int xdg_autostart_service_generate_unit(
                         e_not_show_in);
         }
 
-        r = xdg_autostart_generate_desktop_condition(service, f,
+        r = xdg_autostart_generate_desktop_condition(f,
                                                      "gnome-systemd-autostart-condition",
                                                      service->autostart_condition);
         if (r < 0)
                 return r;
 
-        r = xdg_autostart_generate_desktop_condition(service, f,
+        r = xdg_autostart_generate_desktop_condition(f,
                                                      "kde-systemd-start-condition",
                                                      service->kde_autostart_condition);
         if (r < 0)
                 return r;
 
-        log_debug("%s: symlinking %s in xdg-desktop-autostart.target/.wants…", service->path, service->name);
+        (void) generator_add_symlink(dest, "xdg-desktop-autostart.target", "wants", service->name);
-        return generator_add_symlink(dest, "xdg-desktop-autostart.target", "wants", service->name);
+
+        return 0;
 }

src/xdg-autostart-generator/xdg-autostart-service.h

@@ -26,6 +26,7 @@ typedef struct XdgAutostartService {
 
 } XdgAutostartService;
 
+
 XdgAutostartService * xdg_autostart_service_free(XdgAutostartService *s);
 DEFINE_TRIVIAL_CLEANUP_FUNC(XdgAutostartService*, xdg_autostart_service_free);
 
@@ -33,4 +34,4 @@ char *xdg_autostart_service_translate_name(const char *name);
 int xdg_autostart_format_exec_start(const char *exec, char **ret_exec_start);
 
 XdgAutostartService *xdg_autostart_service_parse_desktop(const char *path);
-int xdg_autostart_service_generate_unit(const XdgAutostartService *service, const char *dest);
+int xdg_autostart_service_generate_unit(XdgAutostartService *service, const char *dest);