[kselftest-next] selftests: cgroup: fix cleanup path in test_memcg_subtree_control()
diff mbox series

Message ID 20190328025659.25684-1-guro@fb.com
State Mainlined
Commit e14d314c7a489f060d6d691866fef5f131281718
Headers show
Series
  • [kselftest-next] selftests: cgroup: fix cleanup path in test_memcg_subtree_control()
Related show

Commit Message

Roman Gushchin March 28, 2019, 2:56 a.m. UTC
Dan reported, that cleanup path in test_memcg_subtree_control()
triggers a static checker warning:
  ./tools/testing/selftests/cgroup/test_memcontrol.c:76 \
  test_memcg_subtree_control()
  error: uninitialized symbol 'child2'.

Fix this by initializing child2 and parent2 variables and
split the cleanup path into few stages.

Signed-off-by: Roman Gushchin <guro@fb.com>
Fixes: 84092dbcf901 ("selftests: cgroup: add memory controller self-tests")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Shuah Khan (Samsung OSG) <shuah@kernel.org>
Cc: Mike Rapoport <rppt@linux.vnet.ibm.com>
---
 .../selftests/cgroup/test_memcontrol.c        | 38 ++++++++++---------
 1 file changed, 21 insertions(+), 17 deletions(-)

Comments

shuah April 8, 2019, 6:53 p.m. UTC | #1
On 3/27/19 8:56 PM, Roman Gushchin wrote:
> Dan reported, that cleanup path in test_memcg_subtree_control()
> triggers a static checker warning:
>    ./tools/testing/selftests/cgroup/test_memcontrol.c:76 \
>    test_memcg_subtree_control()
>    error: uninitialized symbol 'child2'.
> 
> Fix this by initializing child2 and parent2 variables and
> split the cleanup path into few stages.
> 
> Signed-off-by: Roman Gushchin <guro@fb.com>
> Fixes: 84092dbcf901 ("selftests: cgroup: add memory controller self-tests")
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Cc: Dan Carpenter <dan.carpenter@oracle.com>
> Cc: Shuah Khan (Samsung OSG) <shuah@kernel.org>
> Cc: Mike Rapoport <rppt@linux.vnet.ibm.com>


 From and Signed-off don't match. Please fix it and resend the patch.

thanks,
-- Shuah
Roman Gushchin April 8, 2019, 9:46 p.m. UTC | #2
On Mon, Apr 08, 2019 at 12:53:39PM -0600, shuah wrote:
> On 3/27/19 8:56 PM, Roman Gushchin wrote:
> > Dan reported, that cleanup path in test_memcg_subtree_control()
> > triggers a static checker warning:
> >    ./tools/testing/selftests/cgroup/test_memcontrol.c:76 \
> >    test_memcg_subtree_control()
> >    error: uninitialized symbol 'child2'.
> > 
> > Fix this by initializing child2 and parent2 variables and
> > split the cleanup path into few stages.
> > 
> > Signed-off-by: Roman Gushchin <guro@fb.com>
> > Fixes: 84092dbcf901 ("selftests: cgroup: add memory controller self-tests")
> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> > Cc: Dan Carpenter <dan.carpenter@oracle.com>
> > Cc: Shuah Khan (Samsung OSG) <shuah@kernel.org>
> > Cc: Mike Rapoport <rppt@linux.vnet.ibm.com>
> 
> 
> From and Signed-off don't match. Please fix it and resend the patch.

Done.

Thank you!

Roman

Patch
diff mbox series

diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c
index 28d321ba311b..6f339882a6ca 100644
--- a/tools/testing/selftests/cgroup/test_memcontrol.c
+++ b/tools/testing/selftests/cgroup/test_memcontrol.c
@@ -26,7 +26,7 @@ 
  */
 static int test_memcg_subtree_control(const char *root)
 {
-	char *parent, *child, *parent2, *child2;
+	char *parent, *child, *parent2 = NULL, *child2 = NULL;
 	int ret = KSFT_FAIL;
 	char buf[PAGE_SIZE];
 
@@ -34,50 +34,54 @@  static int test_memcg_subtree_control(const char *root)
 	parent = cg_name(root, "memcg_test_0");
 	child = cg_name(root, "memcg_test_0/memcg_test_1");
 	if (!parent || !child)
-		goto cleanup;
+		goto cleanup_free;
 
 	if (cg_create(parent))
-		goto cleanup;
+		goto cleanup_free;
 
 	if (cg_write(parent, "cgroup.subtree_control", "+memory"))
-		goto cleanup;
+		goto cleanup_parent;
 
 	if (cg_create(child))
-		goto cleanup;
+		goto cleanup_parent;
 
 	if (cg_read_strstr(child, "cgroup.controllers", "memory"))
-		goto cleanup;
+		goto cleanup_child;
 
 	/* Create two nested cgroups without enabling memory controller */
 	parent2 = cg_name(root, "memcg_test_1");
 	child2 = cg_name(root, "memcg_test_1/memcg_test_1");
 	if (!parent2 || !child2)
-		goto cleanup;
+		goto cleanup_free2;
 
 	if (cg_create(parent2))
-		goto cleanup;
+		goto cleanup_free2;
 
 	if (cg_create(child2))
-		goto cleanup;
+		goto cleanup_parent2;
 
 	if (cg_read(child2, "cgroup.controllers", buf, sizeof(buf)))
-		goto cleanup;
+		goto cleanup_all;
 
 	if (!cg_read_strstr(child2, "cgroup.controllers", "memory"))
-		goto cleanup;
+		goto cleanup_all;
 
 	ret = KSFT_PASS;
 
-cleanup:
-	cg_destroy(child);
-	cg_destroy(parent);
-	free(parent);
-	free(child);
-
+cleanup_all:
 	cg_destroy(child2);
+cleanup_parent2:
 	cg_destroy(parent2);
+cleanup_free2:
 	free(parent2);
 	free(child2);
+cleanup_child:
+	cg_destroy(child);
+cleanup_parent:
+	cg_destroy(parent);
+cleanup_free:
+	free(parent);
+	free(child);
 
 	return ret;
 }