[LTP] [PATCH] cgroup_fj: Check start_path before removing directories in cleanup

[LTP] [PATCH] cgroup_fj: Check start_path before removing directories in cleanup

[Wake Liu via ltp]

When setup() fails early (for example, when a cgroup controller is not
available and cgroup_require triggers TCONF), start_path has not yet
been assigned.

In this case, cleanup() in cgroup_fj_function.sh unconditionally runs
rmdir "$start_path/ltp_1", which evaluates to rmdir /ltp_1 and fails.
Similarly, common_cleanup() prints a message about removing all ltp
subgroups even when none were created.

Add checks to verify that start_path is non-empty and the directory
exists before attempting removal.

Signed-off-by: Wake Liu <wakel@google.com>
---
 testcases/kernel/controllers/cgroup_fj/cgroup_fj_common.sh | 7 ++++---
 .../kernel/controllers/cgroup_fj/cgroup_fj_function.sh     | 2 +-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/testcases/kernel/controllers/cgroup_fj/cgroup_fj_common.sh b/testcases/kernel/controllers/cgroup_fj/cgroup_fj_common.sh
index 8b5e73466..03e416f55 100755
--- a/testcases/kernel/controllers/cgroup_fj/cgroup_fj_common.sh
+++ b/testcases/kernel/controllers/cgroup_fj/cgroup_fj_common.sh
@@ -79,9 +79,10 @@ common_cleanup()
 {
     killall -9 cgroup_fj_proc >/dev/null 2>&1

-    tst_res TINFO "Removing all ltp subgroups..."
-
-    [ -d "$start_path" ] && find "$start_path" -depth -type d -exec rmdir '{}' \;
+    if [ -n "$start_path" ] && [ -d "$start_path" ]; then
+        tst_res TINFO "Removing all ltp subgroups..."
+        find "$start_path" -depth -type d -exec rmdir '{}' \;
+    fi

     cgroup_cleanup

diff --git a/testcases/kernel/controllers/cgroup_fj/cgroup_fj_function.sh b/testcases/kernel/controllers/cgroup_fj/cgroup_fj_function.sh
index b0c9410ca..61b7a0415 100755
--- a/testcases/kernel/controllers/cgroup_fj/cgroup_fj_function.sh
+++ b/testcases/kernel/controllers/cgroup_fj/cgroup_fj_function.sh
@@ -231,7 +231,7 @@ cleanup()
         wait $pid >/dev/null 2>&1
     fi

-    rmdir "$start_path/ltp_1" >/dev/null 2>&1
+    [ -n "$start_path" ] && [ -d "$start_path/ltp_1" ] && rmdir "$start_path/ltp_1" >/dev/null 2>&1
     common_cleanup
 }

--
2.55.0.897.gb25b4bd76c-goog


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] cgroup_fj: Check start_path before removing directories in cleanup

[linuxtestproject.agent]

Hi Wake,

On August 28, 2026, Wake Liu wrote:
> cgroup_fj: Check start_path before removing directories in cleanup

Verdict - Reviewed

---
Note:

The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.

Regards,
LTP AI Reviewer

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] cgroup_fj: Check start_path before removing directories in cleanup

[Petr Vorel]

Hi Wake Liu,

Thanks, merged with minor changes.

> +++ b/testcases/kernel/controllers/cgroup_fj/cgroup_fj_common.sh @@ -79,9
> +79,10 @@ common_cleanup() { killall -9 cgroup_fj_proc >/dev/null 2>&1

> -    tst_res TINFO "Removing all ltp subgroups..."
> -
> -    [ -d "$start_path" ] && find "$start_path" -depth -type d -exec rmdir '{}' \;
> +    if [ -n "$start_path" ] && [ -d "$start_path" ]; then
> +        tst_res TINFO "Removing all ltp subgroups..."
Let's remove dots.

> +        find "$start_path" -depth -type d -exec rmdir '{}' \;
> +    fi

>      cgroup_cleanup

> diff --git a/testcases/kernel/controllers/cgroup_fj/cgroup_fj_function.sh b/testcases/kernel/controllers/cgroup_fj/cgroup_fj_function.sh
> index b0c9410ca..61b7a0415 100755
> --- a/testcases/kernel/controllers/cgroup_fj/cgroup_fj_function.sh
> +++ b/testcases/kernel/controllers/cgroup_fj/cgroup_fj_function.sh
> @@ -231,7 +231,7 @@ cleanup()
>          wait $pid >/dev/null 2>&1
>      fi

> -    rmdir "$start_path/ltp_1" >/dev/null 2>&1
> +    [ -n "$start_path" ] && [ -d "$start_path/ltp_1" ] && rmdir "$start_path/ltp_1" >/dev/null 2>&1
Now we don't need to mask errors, right?
And a bit long line.

Also, I'm not sure if ... && ... can't bring problems on 'set -e' setup,
because if code is not executed $? gets 1. That's why I use only ... || ...
or if/then/else after which $? is always 0.

>      common_cleanup
>  }

Merged with following diff.

Kind regards,
Petr

+++ testcases/kernel/controllers/cgroup_fj/cgroup_fj_common.sh
@@ -79,9 +79,10 @@ common_cleanup()
 {
     killall -9 cgroup_fj_proc >/dev/null 2>&1
 
-    tst_res TINFO "Removing all ltp subgroups..."
-
-    [ -d "$start_path" ] && find "$start_path" -depth -type d -exec rmdir '{}' \;
+    if [ -n "$start_path" ] && [ -d "$start_path" ]; then
+        tst_res TINFO "Removing all ltp subgroups"
+        find "$start_path" -depth -type d -exec rmdir '{}' \;
+    fi
 
     cgroup_cleanup
 
diff --git testcases/kernel/controllers/cgroup_fj/cgroup_fj_function.sh testcases/kernel/controllers/cgroup_fj/cgroup_fj_function.sh
index b0c9410ca3..2b0dafc544 100755
--- testcases/kernel/controllers/cgroup_fj/cgroup_fj_function.sh
+++ testcases/kernel/controllers/cgroup_fj/cgroup_fj_function.sh
@@ -231,7 +231,10 @@ cleanup()
         wait $pid >/dev/null 2>&1
     fi
 
-    rmdir "$start_path/ltp_1" >/dev/null 2>&1
+    if [ -n "$start_path" ] && [ -d "$start_path/ltp_1" ]; then
+        rmdir "$start_path/ltp_1"
+    fi
+
     common_cleanup
 }
 

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp