From c0ba6b928676acbc6562c28edf820eaf2b5a1d4f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 29 Apr 2019 15:45:19 +0200 Subject: [PATCH 1/2] switch-root: modernize rm_rf_children() invocation a bit --- src/basic/rm-rf.c | 4 ++-- src/shared/switch-root.c | 6 ++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/basic/rm-rf.c b/src/basic/rm-rf.c index acc0f88cd2..3843fc36fc 100644 --- a/src/basic/rm-rf.c +++ b/src/basic/rm-rf.c @@ -33,8 +33,8 @@ int rm_rf_children(int fd, RemoveFlags flags, struct stat *root_dev) { assert(fd >= 0); - /* This returns the first error we run into, but nevertheless - * tries to go on. This closes the passed fd. */ + /* 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.. */ if (!(flags & REMOVE_PHYSICAL)) { diff --git a/src/shared/switch-root.c b/src/shared/switch-root.c index dbb4622312..f721aff760 100644 --- a/src/shared/switch-root.c +++ b/src/shared/switch-root.c @@ -120,10 +120,8 @@ int switch_root(const char *new_root, if (fstat(old_root_fd, &rb) < 0) log_warning_errno(errno, "Failed to stat old root directory, leaving: %m"); - else { - (void) rm_rf_children(old_root_fd, 0, &rb); - old_root_fd = -1; - } + else + (void) rm_rf_children(TAKE_FD(old_root_fd), 0, &rb); /* takes possession of the dir fd, even on failure */ } return 0; From 50b6eec15435ee76855d841f43ee1e1dcb850ef4 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 29 Apr 2019 16:21:01 +0200 Subject: [PATCH 2/2] rm-rf: simplify rm_rf_children() a bit by using _cleanup_close_ --- src/basic/rm-rf.c | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/src/basic/rm-rf.c b/src/basic/rm-rf.c index 3843fc36fc..b751933c83 100644 --- a/src/basic/rm-rf.c +++ b/src/basic/rm-rf.c @@ -85,7 +85,7 @@ int rm_rf_children(int fd, RemoveFlags flags, struct stat *root_dev) { is_dir = de->d_type == DT_DIR; if (is_dir) { - int subdir_fd; + _cleanup_close_ int subdir_fd = -1; /* if root_dev is set, remove subdirectories only if device is same */ if (root_dev && st.st_dev != root_dev->st_dev) @@ -104,13 +104,10 @@ int rm_rf_children(int fd, RemoveFlags flags, struct stat *root_dev) { if (ret == 0 && r != -ENOENT) ret = r; - safe_close(subdir_fd); continue; } - if (r) { - safe_close(subdir_fd); + if (r > 0) continue; - } if ((flags & REMOVE_SUBVOLUME) && st.st_ino == 256) { @@ -122,24 +119,18 @@ int rm_rf_children(int fd, RemoveFlags flags, struct stat *root_dev) { if (ret == 0) ret = r; - safe_close(subdir_fd); continue; } - /* ENOTTY, then it wasn't a - * btrfs subvolume, continue - * below. */ - } else { + /* ENOTTY, then it wasn't a btrfs subvolume, continue below. */ + } else /* It was a subvolume, continue. */ - safe_close(subdir_fd); continue; - } } - /* We pass REMOVE_PHYSICAL here, to avoid - * doing the fstatfs() to check the file + /* We pass REMOVE_PHYSICAL here, to avoid doing the fstatfs() to check the file * system type again for each directory */ - r = rm_rf_children(subdir_fd, flags | REMOVE_PHYSICAL, root_dev); + r = rm_rf_children(TAKE_FD(subdir_fd), flags | REMOVE_PHYSICAL, root_dev); if (r < 0 && ret == 0) ret = r;