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

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

[Disha Goel]

fd is initialized to 0 (a valid file descriptor) so the cleanup()
guard 'if (fd > -1)' would never trigger. Initialize to -1 and
add SAFE_CLOSE to prevent the descriptor leaking when a SAFE_*
macro calls tst_brk() during the pagecache allocation test.

Signed-off-by: Disha Goel <disgoel@linux.ibm.com>
---
 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..75a1e6d51 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 2/2] memcg/memcontrol03-04: fix typos, redundant define and format specifiers

[Disha Goel]

Fix typos in doc comments ('tempfs' -> 'tmpfs', 'becaue' -> 'because',
'dont not' -> 'do not'), 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),
and fix continuation line indentation in tst_res() calls.

Signed-off-by: Disha Goel <disgoel@linux.ibm.com>
---
 testcases/kernel/controllers/memcg/memcontrol03.c | 12 +++++-------
 testcases/kernel/controllers/memcg/memcontrol04.c | 10 ++++------
 2 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/testcases/kernel/controllers/memcg/memcontrol03.c b/testcases/kernel/controllers/memcg/memcontrol03.c
index 493e970ab..42298e5f0 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.
@@ -40,8 +40,6 @@
 
 #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 +103,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 +146,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)
diff --git a/testcases/kernel/controllers/memcg/memcontrol04.c b/testcases/kernel/controllers/memcg/memcontrol04.c
index 15d8f891c..d448478d6 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.
@@ -40,8 +40,6 @@
 
 #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 +100,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 +120,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 +225,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 Fri, Aug 7, 2026, Disha Goel wrote:
> memcg/memcontrol02: fix fd leak in cleanup path

--- [PATCH 1/2] ---

> fd is initialized to 0 (a valid file descriptor) so the cleanup()
> guard 'if (fd > -1)' would never trigger.

Could this explanation be corrected? Zero is greater than -1, and the
cleanup guard is added by this patch rather than being an existing guard.
The relevant problem is that -1 is needed to distinguish an unopened
descriptor before cleanup tries to close it.

> +	if (fd > -1)
> +		SAFE_CLOSE(fd);

Could this use `fd != -1` instead? LTP file descriptors use -1 as the
invalid sentinel, and the LTP convention requires this form for cleanup
guards.

--- [PATCH 2/2] ---

> #include <inttypes.h>
>
> -		tst_res(TINFO, "Child %d in %s: Allocating anon: %"PRIdPTR,
> +		tst_res(TINFO, "Child %d in %s: Allocating anon: %zu",

Could the now-unused `<inttypes.h>` include be removed from both
memcontrol03.c and memcontrol04.c? This patch replaces every `PRIdPTR`
use in both files.

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

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

[Andrea Cervesato via ltp]

Hi Disha,

> > fd is initialized to 0 (a valid file descriptor) so the cleanup()
> > guard 'if (fd > -1)' would never trigger.
> 
> Could this explanation be corrected? Zero is greater than -1, and the
> cleanup guard is added by this patch rather than being an existing guard.
> The relevant problem is that -1 is needed to distinguish an unopened
> descriptor before cleanup tries to close it.
> 
> > +	if (fd > -1)
> > +		SAFE_CLOSE(fd);
> 
> Could this use `fd != -1` instead? LTP file descriptors use -1 as the
> invalid sentinel, and the LTP convention requires this form for cleanup
> guards.

this is correct.

> 
> --- [PATCH 2/2] ---
> 
> > #include <inttypes.h>
> >
> > -		tst_res(TINFO, "Child %d in %s: Allocating anon: %"PRIdPTR,
> > +		tst_res(TINFO, "Child %d in %s: Allocating anon: %zu",
> 
> Could the now-unused `<inttypes.h>` include be removed from both
> memcontrol03.c and memcontrol04.c? This patch replaces every `PRIdPTR`
> use in both files.

And also this is correct. We should split the patch in two: one for
memcontrol03 and one for memcontrol04

--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

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