[bug report] selftests: cgroup: add memory controller self-tests

[bug report] selftests: cgroup: add memory controller self-tests

[dan.carpenter]

Hello Roman Gushchin,

The patch 84092dbcf901: "selftests: cgroup: add memory controller
self-tests" from May 11, 2018, leads to the following static checker
warning:

	./tools/testing/selftests/cgroup/test_memcontrol.c:76 test_memcg_subtree_control()
	error: uninitialized symbol 'child2'.

./tools/testing/selftests/cgroup/test_memcontrol.c
    69  
    70  cleanup:
    71          cg_destroy(child);
    72          cg_destroy(parent);
    73          free(parent);
    74          free(child);
    75  
    76          cg_destroy(child2);

The problem with using one error label to handle all possible returns
is that some stuff hasn't been initialized yet.

    77          cg_destroy(parent2);
    78          free(parent2);
    79          free(child2);
    80  
    81          return ret;
    82  }

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[bug report] selftests: cgroup: add memory controller self-tests

[Dan Carpenter]

Hello Roman Gushchin,

The patch 84092dbcf901: "selftests: cgroup: add memory controller
self-tests" from May 11, 2018, leads to the following static checker
warning:

	./tools/testing/selftests/cgroup/test_memcontrol.c:76 test_memcg_subtree_control()
	error: uninitialized symbol 'child2'.

./tools/testing/selftests/cgroup/test_memcontrol.c
    69  
    70  cleanup:
    71          cg_destroy(child);
    72          cg_destroy(parent);
    73          free(parent);
    74          free(child);
    75  
    76          cg_destroy(child2);

The problem with using one error label to handle all possible returns
is that some stuff hasn't been initialized yet.

    77          cg_destroy(parent2);
    78          free(parent2);
    79          free(child2);
    80  
    81          return ret;
    82  }

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] selftests: cgroup: fix cleanup path in test_memcg_subtree_control()

[guro]

Dan reported, that clenaup 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 at fb.com>
Fixes: 84092dbcf901 ("selftests: cgroup: add memory controller self-tests")
Reported-by: Dan Carpenter <dan.carpenter at oracle.com>
Cc: Shuah Khan (Samsung OSG) <shuah at kernel.org>
Cc: Mike Rapoport <rppt at linux.vnet.ibm.com>
---
 tools/testing/selftests/cgroup/test_memcontrol.c | 38 +++++++++++++-----------
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c
index cf0bddc9d271..ae8f04f84b03 100644
--- a/tools/testing/selftests/cgroup/test_memcontrol.c
+++ b/tools/testing/selftests/cgroup/test_memcontrol.c
@@ -25,7 +25,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];
 
@@ -33,50 +33,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;
 }
-- 
2.14.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] selftests: cgroup: fix cleanup path in test_memcg_subtree_control()

[Roman Gushchin]

Dan reported, that clenaup 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 at fb.com>
Fixes: 84092dbcf901 ("selftests: cgroup: add memory controller self-tests")
Reported-by: Dan Carpenter <dan.carpenter at oracle.com>
Cc: Shuah Khan (Samsung OSG) <shuah at kernel.org>
Cc: Mike Rapoport <rppt at linux.vnet.ibm.com>
---
 tools/testing/selftests/cgroup/test_memcontrol.c | 38 +++++++++++++-----------
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c
index cf0bddc9d271..ae8f04f84b03 100644
--- a/tools/testing/selftests/cgroup/test_memcontrol.c
+++ b/tools/testing/selftests/cgroup/test_memcontrol.c
@@ -25,7 +25,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];
 
@@ -33,50 +33,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;
 }
-- 
2.14.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[bug report] selftests: cgroup: add memory controller self-tests

[guro]

On Tue, Jun 26, 2018 at 12:03:48PM +0300, Dan Carpenter wrote:
> Hello Roman Gushchin,
> 
> The patch 84092dbcf901: "selftests: cgroup: add memory controller
> self-tests" from May 11, 2018, leads to the following static checker
> warning:
> 
> 	./tools/testing/selftests/cgroup/test_memcontrol.c:76 test_memcg_subtree_control()
> 	error: uninitialized symbol 'child2'.
> 
> ./tools/testing/selftests/cgroup/test_memcontrol.c
>     69  
>     70  cleanup:
>     71          cg_destroy(child);
>     72          cg_destroy(parent);
>     73          free(parent);
>     74          free(child);
>     75  
>     76          cg_destroy(child2);
> 
> The problem with using one error label to handle all possible returns
> is that some stuff hasn't been initialized yet.
> 
>     77          cg_destroy(parent2);
>     78          free(parent2);
>     79          free(child2);
>     80  
>     81          return ret;
>     82  }

Hello, Dan!

Thanks for the report!
Just sent the fix.

Thanks,
Roman
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[bug report] selftests: cgroup: add memory controller self-tests

[Roman Gushchin]

On Tue, Jun 26, 2018@12:03:48PM +0300, Dan Carpenter wrote:
> Hello Roman Gushchin,
> 
> The patch 84092dbcf901: "selftests: cgroup: add memory controller
> self-tests" from May 11, 2018, leads to the following static checker
> warning:
> 
> 	./tools/testing/selftests/cgroup/test_memcontrol.c:76 test_memcg_subtree_control()
> 	error: uninitialized symbol 'child2'.
> 
> ./tools/testing/selftests/cgroup/test_memcontrol.c
>     69  
>     70  cleanup:
>     71          cg_destroy(child);
>     72          cg_destroy(parent);
>     73          free(parent);
>     74          free(child);
>     75  
>     76          cg_destroy(child2);
> 
> The problem with using one error label to handle all possible returns
> is that some stuff hasn't been initialized yet.
> 
>     77          cg_destroy(parent2);
>     78          free(parent2);
>     79          free(child2);
>     80  
>     81          return ret;
>     82  }

Hello, Dan!

Thanks for the report!
Just sent the fix.

Thanks,
Roman
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] selftests: cgroup: fix cleanup path in test_memcg_subtree_control()

[guro]

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

diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c
index cf0bddc9d271..ae8f04f84b03 100644
--- a/tools/testing/selftests/cgroup/test_memcontrol.c
+++ b/tools/testing/selftests/cgroup/test_memcontrol.c
@@ -25,7 +25,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];
 
@@ -33,50 +33,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;
 }
-- 
2.14.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] selftests: cgroup: fix cleanup path in test_memcg_subtree_control()

[Roman Gushchin]

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

diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c
index cf0bddc9d271..ae8f04f84b03 100644
--- a/tools/testing/selftests/cgroup/test_memcontrol.c
+++ b/tools/testing/selftests/cgroup/test_memcontrol.c
@@ -25,7 +25,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];
 
@@ -33,50 +33,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;
 }
-- 
2.14.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[bug report] selftests: cgroup: add memory controller self-tests

[guro]

On Wed, Mar 27, 2019 at 04:50:14PM -0600, shuah wrote:
> On 3/27/19 4:24 PM, Roman Gushchin wrote:
> > On Wed, Mar 27, 2019 at 10:32:04AM +0300, Dan Carpenter wrote:
> > > Hello Roman Gushchin,
> > > 
> > > The patch 84092dbcf901: "selftests: cgroup: add memory controller
> > > self-tests" from May 11, 2018, leads to the following static checker
> > > warning:
> > 
> > Hi Dan!
> > 
> > As I remember, this problem has been reported already, and I've
> > proposed a fix for it: https://patchwork.kernel.org/patch/10489785/
> > However, for some reason it hasn't been merged.
> > 
> > Shuakh, can you, please, merge it? Do you want me to respin and resend it?
> > 
> 
> Sorry for the delay!!
> 
> Please rebase and resend to me. I will pull this in.

Just sent. Thank you!

Roman

[bug report] selftests: cgroup: add memory controller self-tests

[Roman Gushchin]

On Wed, Mar 27, 2019@04:50:14PM -0600, shuah wrote:
> On 3/27/19 4:24 PM, Roman Gushchin wrote:
> > On Wed, Mar 27, 2019@10:32:04AM +0300, Dan Carpenter wrote:
> > > Hello Roman Gushchin,
> > > 
> > > The patch 84092dbcf901: "selftests: cgroup: add memory controller
> > > self-tests" from May 11, 2018, leads to the following static checker
> > > warning:
> > 
> > Hi Dan!
> > 
> > As I remember, this problem has been reported already, and I've
> > proposed a fix for it: https://patchwork.kernel.org/patch/10489785/
> > However, for some reason it hasn't been merged.
> > 
> > Shuakh, can you, please, merge it? Do you want me to respin and resend it?
> > 
> 
> Sorry for the delay!!
> 
> Please rebase and resend to me. I will pull this in.

Just sent. Thank you!

Roman

[bug report] selftests: cgroup: add memory controller self-tests

[shuah]

On 3/27/19 4:24 PM, Roman Gushchin wrote:
> On Wed, Mar 27, 2019 at 10:32:04AM +0300, Dan Carpenter wrote:
>> Hello Roman Gushchin,
>>
>> The patch 84092dbcf901: "selftests: cgroup: add memory controller
>> self-tests" from May 11, 2018, leads to the following static checker
>> warning:
> 
> Hi Dan!
> 
> As I remember, this problem has been reported already, and I've
> proposed a fix for it: https://patchwork.kernel.org/patch/10489785/
> However, for some reason it hasn't been merged.
> 
> Shuakh, can you, please, merge it? Do you want me to respin and resend it?
> 

Sorry for the delay!!

Please rebase and resend to me. I will pull this in.

thanks,
-- Shuah

[bug report] selftests: cgroup: add memory controller self-tests

[shuah]

On 3/27/19 4:24 PM, Roman Gushchin wrote:
> On Wed, Mar 27, 2019@10:32:04AM +0300, Dan Carpenter wrote:
>> Hello Roman Gushchin,
>>
>> The patch 84092dbcf901: "selftests: cgroup: add memory controller
>> self-tests" from May 11, 2018, leads to the following static checker
>> warning:
> 
> Hi Dan!
> 
> As I remember, this problem has been reported already, and I've
> proposed a fix for it: https://patchwork.kernel.org/patch/10489785/
> However, for some reason it hasn't been merged.
> 
> Shuakh, can you, please, merge it? Do you want me to respin and resend it?
> 

Sorry for the delay!!

Please rebase and resend to me. I will pull this in.

thanks,
-- Shuah

[bug report] selftests: cgroup: add memory controller self-tests

[guro]

On Wed, Mar 27, 2019 at 10:32:04AM +0300, Dan Carpenter wrote:
> Hello Roman Gushchin,
> 
> The patch 84092dbcf901: "selftests: cgroup: add memory controller
> self-tests" from May 11, 2018, leads to the following static checker
> warning:

Hi Dan!

As I remember, this problem has been reported already, and I've
proposed a fix for it: https://patchwork.kernel.org/patch/10489785/
However, for some reason it hasn't been merged.

Shuakh, can you, please, merge it? Do you want me to respin and resend it?

Thank you!

Roman

> 
> 	./tools/testing/selftests/cgroup/test_memcontrol.c:77 test_memcg_subtree_control()
> 	error: uninitialized symbol 'child2'.
> 
> ./tools/testing/selftests/cgroup/test_memcontrol.c
>     27 static int test_memcg_subtree_control(const char *root)
>     28 {
>     29 	char *parent, *child, *parent2, *child2;
>                                ^^^^^^^^^^^^^^^^
>     30 	int ret = KSFT_FAIL;
>     31 	char buf[PAGE_SIZE];
>     32 
>     33 	/* Create two nested cgroups with the memory controller enabled */
>     34 	parent = cg_name(root, "memcg_test_0");
>     35 	child = cg_name(root, "memcg_test_0/memcg_test_1");
>     36 	if (!parent || !child)
>     37 		goto cleanup;
>                 ^^^^^^^^^^^^
> 
>     38 
>     39 	if (cg_create(parent))
>     40 		goto cleanup;
>     41 
>     42 	if (cg_write(parent, "cgroup.subtree_control", "+memory"))
>     43 		goto cleanup;
>     44 
>     45 	if (cg_create(child))
>     46 		goto cleanup;
>     47 
>     48 	if (cg_read_strstr(child, "cgroup.controllers", "memory"))
>     49 		goto cleanup;
>     50 
>     51 	/* Create two nested cgroups without enabling memory controller */
>     52 	parent2 = cg_name(root, "memcg_test_1");
>     53 	child2 = cg_name(root, "memcg_test_1/memcg_test_1");
>     54 	if (!parent2 || !child2)
>     55 		goto cleanup;
>     56 
>     57 	if (cg_create(parent2))
>     58 		goto cleanup;
>     59 
>     60 	if (cg_create(child2))
>     61 		goto cleanup;
>     62 
>     63 	if (cg_read(child2, "cgroup.controllers", buf, sizeof(buf)))
>     64 		goto cleanup;
>     65 
>     66 	if (!cg_read_strstr(child2, "cgroup.controllers", "memory"))
>     67 		goto cleanup;
>     68 
>     69 	ret = KSFT_PASS;
>     70 
>     71 cleanup:
>     72 	cg_destroy(child);
>     73 	cg_destroy(parent);
>     74 	free(parent);
>     75 	free(child);
>     76 
> --> 77 	cg_destroy(child2);
>         ^^^^^^^^^^^^^^^^^
>     78 	cg_destroy(parent2);
>         ^^^^^^^^^^^^^^^^^^
>     79 	free(parent2);
>     80 	free(child2);
>     81 
>     82 	return ret;
>     83 }
> 
> regards,
> dan carpenter

[bug report] selftests: cgroup: add memory controller self-tests

[Roman Gushchin]

On Wed, Mar 27, 2019@10:32:04AM +0300, Dan Carpenter wrote:
> Hello Roman Gushchin,
> 
> The patch 84092dbcf901: "selftests: cgroup: add memory controller
> self-tests" from May 11, 2018, leads to the following static checker
> warning:

Hi Dan!

As I remember, this problem has been reported already, and I've
proposed a fix for it: https://patchwork.kernel.org/patch/10489785/
However, for some reason it hasn't been merged.

Shuakh, can you, please, merge it? Do you want me to respin and resend it?

Thank you!

Roman

> 
> 	./tools/testing/selftests/cgroup/test_memcontrol.c:77 test_memcg_subtree_control()
> 	error: uninitialized symbol 'child2'.
> 
> ./tools/testing/selftests/cgroup/test_memcontrol.c
>     27 static int test_memcg_subtree_control(const char *root)
>     28 {
>     29 	char *parent, *child, *parent2, *child2;
>                                ^^^^^^^^^^^^^^^^
>     30 	int ret = KSFT_FAIL;
>     31 	char buf[PAGE_SIZE];
>     32 
>     33 	/* Create two nested cgroups with the memory controller enabled */
>     34 	parent = cg_name(root, "memcg_test_0");
>     35 	child = cg_name(root, "memcg_test_0/memcg_test_1");
>     36 	if (!parent || !child)
>     37 		goto cleanup;
>                 ^^^^^^^^^^^^
> 
>     38 
>     39 	if (cg_create(parent))
>     40 		goto cleanup;
>     41 
>     42 	if (cg_write(parent, "cgroup.subtree_control", "+memory"))
>     43 		goto cleanup;
>     44 
>     45 	if (cg_create(child))
>     46 		goto cleanup;
>     47 
>     48 	if (cg_read_strstr(child, "cgroup.controllers", "memory"))
>     49 		goto cleanup;
>     50 
>     51 	/* Create two nested cgroups without enabling memory controller */
>     52 	parent2 = cg_name(root, "memcg_test_1");
>     53 	child2 = cg_name(root, "memcg_test_1/memcg_test_1");
>     54 	if (!parent2 || !child2)
>     55 		goto cleanup;
>     56 
>     57 	if (cg_create(parent2))
>     58 		goto cleanup;
>     59 
>     60 	if (cg_create(child2))
>     61 		goto cleanup;
>     62 
>     63 	if (cg_read(child2, "cgroup.controllers", buf, sizeof(buf)))
>     64 		goto cleanup;
>     65 
>     66 	if (!cg_read_strstr(child2, "cgroup.controllers", "memory"))
>     67 		goto cleanup;
>     68 
>     69 	ret = KSFT_PASS;
>     70 
>     71 cleanup:
>     72 	cg_destroy(child);
>     73 	cg_destroy(parent);
>     74 	free(parent);
>     75 	free(child);
>     76 
> --> 77 	cg_destroy(child2);
>         ^^^^^^^^^^^^^^^^^
>     78 	cg_destroy(parent2);
>         ^^^^^^^^^^^^^^^^^^
>     79 	free(parent2);
>     80 	free(child2);
>     81 
>     82 	return ret;
>     83 }
> 
> regards,
> dan carpenter

[bug report] selftests: cgroup: add memory controller self-tests

[dan.carpenter]

Hello Roman Gushchin,

The patch 84092dbcf901: "selftests: cgroup: add memory controller
self-tests" from May 11, 2018, leads to the following static checker
warning:

	./tools/testing/selftests/cgroup/test_memcontrol.c:77 test_memcg_subtree_control()
	error: uninitialized symbol 'child2'.

./tools/testing/selftests/cgroup/test_memcontrol.c
    27 static int test_memcg_subtree_control(const char *root)
    28 {
    29 	char *parent, *child, *parent2, *child2;
                               ^^^^^^^^^^^^^^^^
    30 	int ret = KSFT_FAIL;
    31 	char buf[PAGE_SIZE];
    32 
    33 	/* Create two nested cgroups with the memory controller enabled */
    34 	parent = cg_name(root, "memcg_test_0");
    35 	child = cg_name(root, "memcg_test_0/memcg_test_1");
    36 	if (!parent || !child)
    37 		goto cleanup;
                ^^^^^^^^^^^^

    38 
    39 	if (cg_create(parent))
    40 		goto cleanup;
    41 
    42 	if (cg_write(parent, "cgroup.subtree_control", "+memory"))
    43 		goto cleanup;
    44 
    45 	if (cg_create(child))
    46 		goto cleanup;
    47 
    48 	if (cg_read_strstr(child, "cgroup.controllers", "memory"))
    49 		goto cleanup;
    50 
    51 	/* Create two nested cgroups without enabling memory controller */
    52 	parent2 = cg_name(root, "memcg_test_1");
    53 	child2 = cg_name(root, "memcg_test_1/memcg_test_1");
    54 	if (!parent2 || !child2)
    55 		goto cleanup;
    56 
    57 	if (cg_create(parent2))
    58 		goto cleanup;
    59 
    60 	if (cg_create(child2))
    61 		goto cleanup;
    62 
    63 	if (cg_read(child2, "cgroup.controllers", buf, sizeof(buf)))
    64 		goto cleanup;
    65 
    66 	if (!cg_read_strstr(child2, "cgroup.controllers", "memory"))
    67 		goto cleanup;
    68 
    69 	ret = KSFT_PASS;
    70 
    71 cleanup:
    72 	cg_destroy(child);
    73 	cg_destroy(parent);
    74 	free(parent);
    75 	free(child);
    76 
--> 77 	cg_destroy(child2);
        ^^^^^^^^^^^^^^^^^
    78 	cg_destroy(parent2);
        ^^^^^^^^^^^^^^^^^^
    79 	free(parent2);
    80 	free(child2);
    81 
    82 	return ret;
    83 }

regards,
dan carpenter

[bug report] selftests: cgroup: add memory controller self-tests

[Dan Carpenter]

Hello Roman Gushchin,

The patch 84092dbcf901: "selftests: cgroup: add memory controller
self-tests" from May 11, 2018, leads to the following static checker
warning:

	./tools/testing/selftests/cgroup/test_memcontrol.c:77 test_memcg_subtree_control()
	error: uninitialized symbol 'child2'.

./tools/testing/selftests/cgroup/test_memcontrol.c
    27 static int test_memcg_subtree_control(const char *root)
    28 {
    29 	char *parent, *child, *parent2, *child2;
                               ^^^^^^^^^^^^^^^^
    30 	int ret = KSFT_FAIL;
    31 	char buf[PAGE_SIZE];
    32 
    33 	/* Create two nested cgroups with the memory controller enabled */
    34 	parent = cg_name(root, "memcg_test_0");
    35 	child = cg_name(root, "memcg_test_0/memcg_test_1");
    36 	if (!parent || !child)
    37 		goto cleanup;
                ^^^^^^^^^^^^

    38 
    39 	if (cg_create(parent))
    40 		goto cleanup;
    41 
    42 	if (cg_write(parent, "cgroup.subtree_control", "+memory"))
    43 		goto cleanup;
    44 
    45 	if (cg_create(child))
    46 		goto cleanup;
    47 
    48 	if (cg_read_strstr(child, "cgroup.controllers", "memory"))
    49 		goto cleanup;
    50 
    51 	/* Create two nested cgroups without enabling memory controller */
    52 	parent2 = cg_name(root, "memcg_test_1");
    53 	child2 = cg_name(root, "memcg_test_1/memcg_test_1");
    54 	if (!parent2 || !child2)
    55 		goto cleanup;
    56 
    57 	if (cg_create(parent2))
    58 		goto cleanup;
    59 
    60 	if (cg_create(child2))
    61 		goto cleanup;
    62 
    63 	if (cg_read(child2, "cgroup.controllers", buf, sizeof(buf)))
    64 		goto cleanup;
    65 
    66 	if (!cg_read_strstr(child2, "cgroup.controllers", "memory"))
    67 		goto cleanup;
    68 
    69 	ret = KSFT_PASS;
    70 
    71 cleanup:
    72 	cg_destroy(child);
    73 	cg_destroy(parent);
    74 	free(parent);
    75 	free(child);
    76 
--> 77 	cg_destroy(child2);
        ^^^^^^^^^^^^^^^^^
    78 	cg_destroy(parent2);
        ^^^^^^^^^^^^^^^^^^
    79 	free(parent2);
    80 	free(child2);
    81 
    82 	return ret;
    83 }

regards,
dan carpenter

[bug report] selftests: cgroup: add memory controller self-tests

[guro]

On Thu, May 17, 2018 at 04:22:25PM +0300, Dan Carpenter wrote:

> Hello Roman Gushchin,
> 
> The patch a62213fe9b77: "selftests: cgroup: add memory controller
> self-tests" from May 11, 2018, leads to the following static checker
> warning:
> 
> 	./tools/testing/selftests/cgroup/cgroup_util.c:62 cg_name()
> 	warn: variable dereferenced before check 'name' (see line 59)
> 

Hi Dan!

Thank you for the report!
The fix is below.


Roman

--

>From ec990dc816b0a36201bed84e5b38b928b9bb9138 Mon Sep 17 00:00:00 2001
From: Roman Gushchin <guro at fb.com>
Date: Thu, 17 May 2018 16:53:15 +0100
Subject: [PATCH] kselftest/cgroup: fix variable dereferenced before check
 warning

cg_name(const char *root, const char *name) is always called with
non-empty root and name arguments, so there is no sense in checking
it in the function body (after using in strlen()).

Signed-off-by: Roman Gushchin <guro at fb.com>
Reported-by: Dan Carpenter <dan.carpenter at oracle.com>
Cc: linux-kselftest at vger.kernel.org
---
 tools/testing/selftests/cgroup/cgroup_util.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/cgroup/cgroup_util.c b/tools/testing/selftests/cgroup/cgroup_util.c
index 41cc3b5e5be1..b69bdeb4b9fe 100644
--- a/tools/testing/selftests/cgroup/cgroup_util.c
+++ b/tools/testing/selftests/cgroup/cgroup_util.c
@@ -59,8 +59,7 @@ char *cg_name(const char *root, const char *name)
 	size_t len = strlen(root) + strlen(name) + 2;
 	char *ret = malloc(len);
 
-	if (name)
-		snprintf(ret, len, "%s/%s", root, name);
+	snprintf(ret, len, "%s/%s", root, name);
 
 	return ret;
 }
@@ -70,8 +69,7 @@ char *cg_name_indexed(const char *root, const char *name, int index)
 	size_t len = strlen(root) + strlen(name) + 10;
 	char *ret = malloc(len);
 
-	if (name)
-		snprintf(ret, len, "%s/%s_%d", root, name, index);
+	snprintf(ret, len, "%s/%s_%d", root, name, index);
 
 	return ret;
 }
-- 
2.14.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[bug report] selftests: cgroup: add memory controller self-tests

[Roman Gushchin]

On Thu, May 17, 2018@04:22:25PM +0300, Dan Carpenter wrote:

> Hello Roman Gushchin,
> 
> The patch a62213fe9b77: "selftests: cgroup: add memory controller
> self-tests" from May 11, 2018, leads to the following static checker
> warning:
> 
> 	./tools/testing/selftests/cgroup/cgroup_util.c:62 cg_name()
> 	warn: variable dereferenced before check 'name' (see line 59)
> 

Hi Dan!

Thank you for the report!
The fix is below.


Roman

--

>From ec990dc816b0a36201bed84e5b38b928b9bb9138 Mon Sep 17 00:00:00 2001
From: Roman Gushchin <guro@fb.com>
Date: Thu, 17 May 2018 16:53:15 +0100
Subject: [PATCH] kselftest/cgroup: fix variable dereferenced before check
 warning

cg_name(const char *root, const char *name) is always called with
non-empty root and name arguments, so there is no sense in checking
it in the function body (after using in strlen()).

Signed-off-by: Roman Gushchin <guro at fb.com>
Reported-by: Dan Carpenter <dan.carpenter at oracle.com>
Cc: linux-kselftest at vger.kernel.org
---
 tools/testing/selftests/cgroup/cgroup_util.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/cgroup/cgroup_util.c b/tools/testing/selftests/cgroup/cgroup_util.c
index 41cc3b5e5be1..b69bdeb4b9fe 100644
--- a/tools/testing/selftests/cgroup/cgroup_util.c
+++ b/tools/testing/selftests/cgroup/cgroup_util.c
@@ -59,8 +59,7 @@ char *cg_name(const char *root, const char *name)
 	size_t len = strlen(root) + strlen(name) + 2;
 	char *ret = malloc(len);
 
-	if (name)
-		snprintf(ret, len, "%s/%s", root, name);
+	snprintf(ret, len, "%s/%s", root, name);
 
 	return ret;
 }
@@ -70,8 +69,7 @@ char *cg_name_indexed(const char *root, const char *name, int index)
 	size_t len = strlen(root) + strlen(name) + 10;
 	char *ret = malloc(len);
 
-	if (name)
-		snprintf(ret, len, "%s/%s_%d", root, name, index);
+	snprintf(ret, len, "%s/%s_%d", root, name, index);
 
 	return ret;
 }
-- 
2.14.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[bug report] selftests: cgroup: add memory controller self-tests

[dan.carpenter]

Hello Roman Gushchin,

The patch a62213fe9b77: "selftests: cgroup: add memory controller
self-tests" from May 11, 2018, leads to the following static checker
warning:

	./tools/testing/selftests/cgroup/cgroup_util.c:62 cg_name()
	warn: variable dereferenced before check 'name' (see line 59)

./tools/testing/selftests/cgroup/cgroup_util.c
    57  char *cg_name(const char *root, const char *name)
    58  {
    59          size_t len = strlen(root) + strlen(name) + 2;
                                            ^^^^^^^^^^^^
    60          char *ret = malloc(len);
    61  
    62          if (name)
                    ^^^^
    63                  snprintf(ret, len, "%s/%s", root, name);
    64  
    65          return ret;
    66  }
    67  
    68  char *cg_name_indexed(const char *root, const char *name, int index)
    69  {
    70          size_t len = strlen(root) + strlen(name) + 10;
                                            ^^^^^^^^^^^^
    71          char *ret = malloc(len);
    72  
    73          if (name)
                    ^^^^
    74                  snprintf(ret, len, "%s/%s_%d", root, name, index);
    75  
    76          return ret;
    77  }

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[bug report] selftests: cgroup: add memory controller self-tests

[Dan Carpenter]

Hello Roman Gushchin,

The patch a62213fe9b77: "selftests: cgroup: add memory controller
self-tests" from May 11, 2018, leads to the following static checker
warning:

	./tools/testing/selftests/cgroup/cgroup_util.c:62 cg_name()
	warn: variable dereferenced before check 'name' (see line 59)

./tools/testing/selftests/cgroup/cgroup_util.c
    57  char *cg_name(const char *root, const char *name)
    58  {
    59          size_t len = strlen(root) + strlen(name) + 2;
                                            ^^^^^^^^^^^^
    60          char *ret = malloc(len);
    61  
    62          if (name)
                    ^^^^
    63                  snprintf(ret, len, "%s/%s", root, name);
    64  
    65          return ret;
    66  }
    67  
    68  char *cg_name_indexed(const char *root, const char *name, int index)
    69  {
    70          size_t len = strlen(root) + strlen(name) + 10;
                                            ^^^^^^^^^^^^
    71          char *ret = malloc(len);
    72  
    73          if (name)
                    ^^^^
    74                  snprintf(ret, len, "%s/%s_%d", root, name, index);
    75  
    76          return ret;
    77  }

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html