[LTP] [PATCH v2 1/3] memcg/memcontrol02: fix fd leak in cleanup path

[LTP] [PATCH v2 1/3] memcg/memcontrol02: fix fd leak in cleanup path

[Disha Goel]

Initialize fd to -1 to distinguish an unopened descriptor, and add
a cleanup guard so SAFE_CLOSE is only called when fd was actually
opened. Without this, cleanup would call close(-1) causing TBROK
when the anon test variant (n=0) runs and never opens fd.

Signed-off-by: Disha Goel <disgoel@linux.ibm.com>
---
v1 -> v2:
- Fix commit message: clarify that fd=0 would close stdin (not silently
  skip), and that the guard is added by this patch not pre-existing
- Use 'fd != -1' instead of 'fd > -1' per LTP convention

 testcases/kernel/controllers/memcg/memcontrol02.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/controllers/memcg/memcontrol02.c b/testcases/kernel/controllers/memcg/memcontrol02.c
index 0b79403c9..52096f697 100644
--- a/testcases/kernel/controllers/memcg/memcontrol02.c
+++ b/testcases/kernel/controllers/memcg/memcontrol02.c
@@ -29,7 +29,7 @@
 
 static size_t page_size;
 static struct tst_cg_group *cg_child;
-static int fd;
+static int fd = -1;
 static int file_to_all_error = 10;
 
 static void alloc_anon_50M_check(void)
@@ -128,6 +128,8 @@ static void cleanup(void)
 {
 	if (cg_child)
 		cg_child = tst_cg_group_rm(cg_child);
+	if (fd != -1)
+		SAFE_CLOSE(fd);
 }
 
 static struct tst_test test = {
-- 
2.45.1


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

[LTP] [PATCH v2 2/3] memcg/memcontrol03: fix typos, redundant define and format specifiers

[Disha Goel]

Fix typos in doc comment ('tempfs' -> 'tmpfs', 'becaue' -> 'because'),
remove '#define TMPDIR \"mntdir\"' which duplicates the definition already
in memcontrol_common.h, replace PRIdPTR with %zu for size_t arguments
(PRIdPTR is for signed intptr_t, not unsigned size_t), fix continuation
line indentation in tst_res() calls, and remove now-unused <inttypes.h>
include.

Signed-off-by: Disha Goel <disgoel@linux.ibm.com>
---
v1 -> v2:
- Remove now-unused <inttypes.h> include (PRIdPTR was its only user)
- Split from combined memcontrol03+04 patch into separate patch per file

 testcases/kernel/controllers/memcg/memcontrol03.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/testcases/kernel/controllers/memcg/memcontrol03.c b/testcases/kernel/controllers/memcg/memcontrol03.c
index 493e970ab..96224c4ee 100644
--- a/testcases/kernel/controllers/memcg/memcontrol03.c
+++ b/testcases/kernel/controllers/memcg/memcontrol03.c
@@ -27,8 +27,8 @@
  * pagecache even in this case."
  *
  * memory.min doesn't appear to exist on V1 so we only test on V2 like
- * the selftest. We do test on more file systems, but not tempfs
- * becaue it can't evict the page cache without swap. Also we avoid
+ * the selftest. We do test on more file systems, but not tmpfs
+ * because it can't evict the page cache without swap. Also we avoid
  * filesystems which allocate extra memory for buffer heads.
  *
  * The tolerances have been increased from the self tests.
@@ -36,12 +36,8 @@
 
 #define _GNU_SOURCE
 
-#include <inttypes.h>
-
 #include "memcontrol_common.h"
 
-#define TMPDIR "mntdir"
-
 static struct tst_cg_group *trunk_cg[3];
 static struct tst_cg_group *leaf_cg[4];
 static int fd = -1;
@@ -105,8 +101,8 @@ static void alloc_anon_in_child(const struct tst_cg_group *const cg,
 		SAFE_CG_SCANF(cg, "memory.current", "%zu", &cgmem);
 		size = size > cgmem ? size - cgmem : 0;
 
-		tst_res(TINFO, "Child %d in %s: Allocating anon: %"PRIdPTR,
-		getpid(), tst_cg_group_name(cg), size);
+		tst_res(TINFO, "Child %d in %s: Allocating anon: %zu",
+			getpid(), tst_cg_group_name(cg), size);
 
 		if (size)
 			alloc_anon(size);
@@ -148,7 +144,7 @@ static void alloc_pagecache_in_child(const struct tst_cg_group *const cg,
 	SAFE_CG_SCANF(cg, "memory.current", "%zu", &cgmem);
 	size = size > cgmem ? size - cgmem : 0;
 
-	tst_res(TINFO, "Child %d in %s: Allocating pagecache: %"PRIdPTR,
+	tst_res(TINFO, "Child %d in %s: Allocating pagecache: %zu",
 		getpid(), tst_cg_group_name(cg), size);
 
 	if (size)
-- 
2.45.1


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

[LTP] [PATCH v2 3/3] memcg/memcontrol04: fix typos, redundant define and format specifiers

[Disha Goel]

Fix typos in doc comment ('tempfs' -> 'tmpfs', 'becaue' -> 'because'),
remove '#define TMPDIR \"mntdir\"' which duplicates the definition already
in memcontrol_common.h, replace PRIdPTR with %zu for size_t arguments
(PRIdPTR is for signed intptr_t, not unsigned size_t), fix 'dont not'
-> 'do not' in inline comment, and remove now-unused <inttypes.h>
include.

Signed-off-by: Disha Goel <disgoel@linux.ibm.com>
---
v1 -> v2:
- Remove now-unused <inttypes.h> include (PRIdPTR was its only user)
- Split from combined memcontrol03+04 patch into separate patch per file

 testcases/kernel/controllers/memcg/memcontrol04.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/testcases/kernel/controllers/memcg/memcontrol04.c b/testcases/kernel/controllers/memcg/memcontrol04.c
index 15d8f891c..22ca1df8c 100644
--- a/testcases/kernel/controllers/memcg/memcontrol04.c
+++ b/testcases/kernel/controllers/memcg/memcontrol04.c
@@ -28,7 +28,7 @@
  * The closest thing to memory.low on V1 is soft_limit_in_bytes which
  * uses a different mechanism and has different semantics. So we only
  * test on V2 like the selftest. We do test on more file systems, but
- * not tempfs becaue it can't evict the page cache without swap. Also
+ * not tmpfs because it can't evict the page cache without swap. Also
  * we avoid filesystems which allocate extra memory for buffer heads.
  *
  * The tolerances have been increased from the self tests.
@@ -36,12 +36,8 @@
 
 #define _GNU_SOURCE
 
-#include <inttypes.h>
-
 #include "memcontrol_common.h"
 
-#define TMPDIR "mntdir"
-
 static struct tst_cg_group *trunk_cg[3];
 static struct tst_cg_group *leaf_cg[4];
 static int fd = -1;
@@ -102,7 +98,7 @@ static void alloc_anon_in_child(const struct tst_cg_group *const cg,
 
 	SAFE_CG_PRINTF(cg, "cgroup.procs", "%d", getpid());
 
-	tst_res(TINFO, "Child %d in %s: Allocating anon: %"PRIdPTR,
+	tst_res(TINFO, "Child %d in %s: Allocating anon: %zu",
 		getpid(), tst_cg_group_name(cg), size);
 	alloc_anon(size);
 
@@ -122,7 +118,7 @@ static void alloc_pagecache_in_child(const struct tst_cg_group *const cg,
 
 	SAFE_CG_PRINTF(cg, "cgroup.procs", "%d", getpid());
 
-	tst_res(TINFO, "Child %d in %s: Allocating pagecache: %"PRIdPTR,
+	tst_res(TINFO, "Child %d in %s: Allocating pagecache: %zu",
 		getpid(), tst_cg_group_name(cg), size);
 	alloc_pagecache(fd, size);
 
@@ -227,7 +223,7 @@ static void test_memcg_low(void)
 			TST_EXP_EXPR(low == 0,
 				"(%c low events=%ld) == 0", id, low);
 		} else if (!tst_cg_memory_recursiveprot(leaf_cg[F])) {
-			/* dont not check F when recursive_protection enabled */
+			/* do not check F when recursive_protection enabled */
 			TST_EXP_EXPR(low == 0,
 				"(%c low events=%ld) == 0", id, low);
 		}
-- 
2.45.1


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

Re: [LTP] memcg/memcontrol02: fix fd leak in cleanup path

[linuxtestproject.agent]

Hi Disha,

On Aug 18, 2026, Disha Goel wrote:
> memcg/memcontrol02: fix fd leak in cleanup path

--- [PATCH 1/3] ---

> Without this, cleanup would call close(-1) causing TBROK
> when the anon test variant (n=0) runs and never opens fd.

Could this describe the actual failure path instead? Before this patch,
cleanup() does not call close() at all. The descriptor is leaked when an
error after SAFE_OPEN() skips the normal SAFE_CLOSE(fd); the -1 guard is
needed only after adding the cleanup close.

Verdict - Needs revision

---
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