Re: [PATCH 1/2] Fix cg_read_strcmp()
On 09/07/2018 12:28 PM, Jay Kamat wrote:
>
> Shuah Khan writes:
>
>> On 09/07/2018 10:49 AM, jgka...@fb.com wrote:
>>> From: Jay Kamat
>>>
>>> Fix a couple issues with cg_read_strcmp(), to improve correctness of
>>> cgroup tests
>>> - Fix cg_read_strcmp() always returning 0 for empty "needle" strings
>>> - Fix a memory leak in cg_read_strcmp()
>>>
>>> Fixes: 84092dbcf901 ("selftests: cgroup: add memory controller self-tests")
>>>
>>> Signed-off-by: Jay Kamat
>>> ---
>>> tools/testing/selftests/cgroup/cgroup_util.c | 17 ++---
>>> 1 file changed, 14 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/cgroup/cgroup_util.c
>>> b/tools/testing/selftests/cgroup/cgroup_util.c
>>> index 1e9e3c470561..8b644ea39725 100644
>>> --- a/tools/testing/selftests/cgroup/cgroup_util.c
>>> +++ b/tools/testing/selftests/cgroup/cgroup_util.c
>>> @@ -89,17 +89,28 @@ int cg_read(const char *cgroup, const char *control,
>>> char *buf, size_t len)
>>> int cg_read_strcmp(const char *cgroup, const char *control,
>>>const char *expected)
>>> {
>>> - size_t size = strlen(expected) + 1;
>>> + size_t size;
>>> char *buf;
>>> + int ret;
>>> +
>>> + /* Handle the case of comparing against empty string */
>>> + if (!expected)
>>> + size = 32;
>>
>> This doesn't look right. I would think expected shouldn't be null?
>> It gets used below.
>>
>>> + else
>>> + size = strlen(expected) + 1;
>>>
>>> buf = malloc(size);
>>> if (!buf)
>>> return -1;
>>>
>>> - if (cg_read(cgroup, control, buf, size))
>>> + if (cg_read(cgroup, control, buf, size)) {
>>> + free(buf);
>>> return -1;
>>> + }
>>>
>>> - return strcmp(expected, buf);
>>> + ret = strcmp(expected, buf);
>>
>> If expected is null, what's the point in running the test?
>> Is empty "needle" string a valid test scenario?
>
> There are a couple places where an empty "needle" string is used currently:
>
> - cg_test_proc_killed (newly added in the next patch): Verify cgroup.procs is
> empty (there are no processes running)
> - test_memcg_oom_events: Verify cgroup.procs is empty
Yes I see the empty neede string usage now.
>
> Previously, when passing in an empty needle string, this function would always
> return 0, as the size allocated (1) would not be enough to read any data in
> 'cg_read', and strcmp would compare two null strings.
Thanks for explaining this. Yes this fix is good. This would be good information
to add to the change log.
Could you please add this to the change log and send v2. I will pull these in
for
the next rc.
thanks,
-- Shuah
Re: [PATCH 1/2] Fix cg_read_strcmp()
On 09/07/2018 12:28 PM, Jay Kamat wrote:
>
> Shuah Khan writes:
>
>> On 09/07/2018 10:49 AM, jgka...@fb.com wrote:
>>> From: Jay Kamat
>>>
>>> Fix a couple issues with cg_read_strcmp(), to improve correctness of
>>> cgroup tests
>>> - Fix cg_read_strcmp() always returning 0 for empty "needle" strings
>>> - Fix a memory leak in cg_read_strcmp()
>>>
>>> Fixes: 84092dbcf901 ("selftests: cgroup: add memory controller self-tests")
>>>
>>> Signed-off-by: Jay Kamat
>>> ---
>>> tools/testing/selftests/cgroup/cgroup_util.c | 17 ++---
>>> 1 file changed, 14 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/cgroup/cgroup_util.c
>>> b/tools/testing/selftests/cgroup/cgroup_util.c
>>> index 1e9e3c470561..8b644ea39725 100644
>>> --- a/tools/testing/selftests/cgroup/cgroup_util.c
>>> +++ b/tools/testing/selftests/cgroup/cgroup_util.c
>>> @@ -89,17 +89,28 @@ int cg_read(const char *cgroup, const char *control,
>>> char *buf, size_t len)
>>> int cg_read_strcmp(const char *cgroup, const char *control,
>>>const char *expected)
>>> {
>>> - size_t size = strlen(expected) + 1;
>>> + size_t size;
>>> char *buf;
>>> + int ret;
>>> +
>>> + /* Handle the case of comparing against empty string */
>>> + if (!expected)
>>> + size = 32;
>>
>> This doesn't look right. I would think expected shouldn't be null?
>> It gets used below.
>>
>>> + else
>>> + size = strlen(expected) + 1;
>>>
>>> buf = malloc(size);
>>> if (!buf)
>>> return -1;
>>>
>>> - if (cg_read(cgroup, control, buf, size))
>>> + if (cg_read(cgroup, control, buf, size)) {
>>> + free(buf);
>>> return -1;
>>> + }
>>>
>>> - return strcmp(expected, buf);
>>> + ret = strcmp(expected, buf);
>>
>> If expected is null, what's the point in running the test?
>> Is empty "needle" string a valid test scenario?
>
> There are a couple places where an empty "needle" string is used currently:
>
> - cg_test_proc_killed (newly added in the next patch): Verify cgroup.procs is
> empty (there are no processes running)
> - test_memcg_oom_events: Verify cgroup.procs is empty
Yes I see the empty neede string usage now.
>
> Previously, when passing in an empty needle string, this function would always
> return 0, as the size allocated (1) would not be enough to read any data in
> 'cg_read', and strcmp would compare two null strings.
Thanks for explaining this. Yes this fix is good. This would be good information
to add to the change log.
Could you please add this to the change log and send v2. I will pull these in
for
the next rc.
thanks,
-- Shuah
Re: [PATCH 1/2] Fix cg_read_strcmp()
Shuah Khan writes:
> On 09/07/2018 10:49 AM, jgka...@fb.com wrote:
>> From: Jay Kamat
>>
>> Fix a couple issues with cg_read_strcmp(), to improve correctness of
>> cgroup tests
>> - Fix cg_read_strcmp() always returning 0 for empty "needle" strings
>> - Fix a memory leak in cg_read_strcmp()
>>
>> Fixes: 84092dbcf901 ("selftests: cgroup: add memory controller self-tests")
>>
>> Signed-off-by: Jay Kamat
>> ---
>> tools/testing/selftests/cgroup/cgroup_util.c | 17 ++---
>> 1 file changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/testing/selftests/cgroup/cgroup_util.c
>> b/tools/testing/selftests/cgroup/cgroup_util.c
>> index 1e9e3c470561..8b644ea39725 100644
>> --- a/tools/testing/selftests/cgroup/cgroup_util.c
>> +++ b/tools/testing/selftests/cgroup/cgroup_util.c
>> @@ -89,17 +89,28 @@ int cg_read(const char *cgroup, const char *control,
>> char *buf, size_t len)
>> int cg_read_strcmp(const char *cgroup, const char *control,
>> const char *expected)
>> {
>> -size_t size = strlen(expected) + 1;
>> +size_t size;
>> char *buf;
>> +int ret;
>> +
>> +/* Handle the case of comparing against empty string */
>> +if (!expected)
>> +size = 32;
>
> This doesn't look right. I would think expected shouldn't be null?
> It gets used below.
>
>> +else
>> +size = strlen(expected) + 1;
>>
>> buf = malloc(size);
>> if (!buf)
>> return -1;
>>
>> -if (cg_read(cgroup, control, buf, size))
>> +if (cg_read(cgroup, control, buf, size)) {
>> +free(buf);
>> return -1;
>> +}
>>
>> -return strcmp(expected, buf);
>> +ret = strcmp(expected, buf);
>
> If expected is null, what's the point in running the test?
> Is empty "needle" string a valid test scenario?
There are a couple places where an empty "needle" string is used currently:
- cg_test_proc_killed (newly added in the next patch): Verify cgroup.procs is
empty (there are no processes running)
- test_memcg_oom_events: Verify cgroup.procs is empty
Previously, when passing in an empty needle string, this function would always
return 0, as the size allocated (1) would not be enough to read any data in
'cg_read', and strcmp would compare two null strings.
>
>> +free(buf);
>> +return ret;
>> }
>>
>> int cg_read_strstr(const char *cgroup, const char *control, const char
>> *needle)
>>
>
> thanks,
> -- Shuah
I could definitely remove the unneeded strcmp in the null 'expected' case, but
I am worried it would feel a bit too hacky or add too much duplication.
Would something like this be the best solution? If you had something else in
mind (or if I'm misunderstanding something), please let me know, and I'll
update the patchset!
size_t size;
char *buf;
int ret;
/* Handle the case of comparing against empty string */
if (!expected)
size = 32;
else
size = strlen(expected) + 1;
buf = malloc(size);
if (!buf)
return -1;
if (cg_read(cgroup, control, buf, size)) {
free(buf);
return -1;
}
if (!expected)
ret = !buf;
else
ret = strcmp(expected, buf);
free(buf);
return ret;
Thanks,
-Jay
Re: [PATCH 1/2] Fix cg_read_strcmp()
Shuah Khan writes:
> On 09/07/2018 10:49 AM, jgka...@fb.com wrote:
>> From: Jay Kamat
>>
>> Fix a couple issues with cg_read_strcmp(), to improve correctness of
>> cgroup tests
>> - Fix cg_read_strcmp() always returning 0 for empty "needle" strings
>> - Fix a memory leak in cg_read_strcmp()
>>
>> Fixes: 84092dbcf901 ("selftests: cgroup: add memory controller self-tests")
>>
>> Signed-off-by: Jay Kamat
>> ---
>> tools/testing/selftests/cgroup/cgroup_util.c | 17 ++---
>> 1 file changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/testing/selftests/cgroup/cgroup_util.c
>> b/tools/testing/selftests/cgroup/cgroup_util.c
>> index 1e9e3c470561..8b644ea39725 100644
>> --- a/tools/testing/selftests/cgroup/cgroup_util.c
>> +++ b/tools/testing/selftests/cgroup/cgroup_util.c
>> @@ -89,17 +89,28 @@ int cg_read(const char *cgroup, const char *control,
>> char *buf, size_t len)
>> int cg_read_strcmp(const char *cgroup, const char *control,
>> const char *expected)
>> {
>> -size_t size = strlen(expected) + 1;
>> +size_t size;
>> char *buf;
>> +int ret;
>> +
>> +/* Handle the case of comparing against empty string */
>> +if (!expected)
>> +size = 32;
>
> This doesn't look right. I would think expected shouldn't be null?
> It gets used below.
>
>> +else
>> +size = strlen(expected) + 1;
>>
>> buf = malloc(size);
>> if (!buf)
>> return -1;
>>
>> -if (cg_read(cgroup, control, buf, size))
>> +if (cg_read(cgroup, control, buf, size)) {
>> +free(buf);
>> return -1;
>> +}
>>
>> -return strcmp(expected, buf);
>> +ret = strcmp(expected, buf);
>
> If expected is null, what's the point in running the test?
> Is empty "needle" string a valid test scenario?
There are a couple places where an empty "needle" string is used currently:
- cg_test_proc_killed (newly added in the next patch): Verify cgroup.procs is
empty (there are no processes running)
- test_memcg_oom_events: Verify cgroup.procs is empty
Previously, when passing in an empty needle string, this function would always
return 0, as the size allocated (1) would not be enough to read any data in
'cg_read', and strcmp would compare two null strings.
>
>> +free(buf);
>> +return ret;
>> }
>>
>> int cg_read_strstr(const char *cgroup, const char *control, const char
>> *needle)
>>
>
> thanks,
> -- Shuah
I could definitely remove the unneeded strcmp in the null 'expected' case, but
I am worried it would feel a bit too hacky or add too much duplication.
Would something like this be the best solution? If you had something else in
mind (or if I'm misunderstanding something), please let me know, and I'll
update the patchset!
size_t size;
char *buf;
int ret;
/* Handle the case of comparing against empty string */
if (!expected)
size = 32;
else
size = strlen(expected) + 1;
buf = malloc(size);
if (!buf)
return -1;
if (cg_read(cgroup, control, buf, size)) {
free(buf);
return -1;
}
if (!expected)
ret = !buf;
else
ret = strcmp(expected, buf);
free(buf);
return ret;
Thanks,
-Jay
Re: [PATCH 1/2] Fix cg_read_strcmp()
On 09/07/2018 10:49 AM, jgka...@fb.com wrote:
> From: Jay Kamat
>
> Fix a couple issues with cg_read_strcmp(), to improve correctness of
> cgroup tests
> - Fix cg_read_strcmp() always returning 0 for empty "needle" strings
> - Fix a memory leak in cg_read_strcmp()
>
> Fixes: 84092dbcf901 ("selftests: cgroup: add memory controller self-tests")
>
> Signed-off-by: Jay Kamat
> ---
> tools/testing/selftests/cgroup/cgroup_util.c | 17 ++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/cgroup/cgroup_util.c
> b/tools/testing/selftests/cgroup/cgroup_util.c
> index 1e9e3c470561..8b644ea39725 100644
> --- a/tools/testing/selftests/cgroup/cgroup_util.c
> +++ b/tools/testing/selftests/cgroup/cgroup_util.c
> @@ -89,17 +89,28 @@ int cg_read(const char *cgroup, const char *control, char
> *buf, size_t len)
> int cg_read_strcmp(const char *cgroup, const char *control,
> const char *expected)
> {
> - size_t size = strlen(expected) + 1;
> + size_t size;
> char *buf;
> + int ret;
> +
> + /* Handle the case of comparing against empty string */
> + if (!expected)
> + size = 32;
This doesn't look right. I would think expected shouldn't be null?
It gets used below.
> + else
> + size = strlen(expected) + 1;
>
> buf = malloc(size);
> if (!buf)
> return -1;
>
> - if (cg_read(cgroup, control, buf, size))
> + if (cg_read(cgroup, control, buf, size)) {
> + free(buf);
> return -1;
> + }
>
> - return strcmp(expected, buf);
> + ret = strcmp(expected, buf);
If expected is null, what's the point in running the test?
Is empty "needle" string a valid test scenario?
> + free(buf);
> + return ret;
> }
>
> int cg_read_strstr(const char *cgroup, const char *control, const char
> *needle)
>
thanks,
-- Shuah
Re: [PATCH 1/2] Fix cg_read_strcmp()
On 09/07/2018 10:49 AM, jgka...@fb.com wrote:
> From: Jay Kamat
>
> Fix a couple issues with cg_read_strcmp(), to improve correctness of
> cgroup tests
> - Fix cg_read_strcmp() always returning 0 for empty "needle" strings
> - Fix a memory leak in cg_read_strcmp()
>
> Fixes: 84092dbcf901 ("selftests: cgroup: add memory controller self-tests")
>
> Signed-off-by: Jay Kamat
> ---
> tools/testing/selftests/cgroup/cgroup_util.c | 17 ++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/cgroup/cgroup_util.c
> b/tools/testing/selftests/cgroup/cgroup_util.c
> index 1e9e3c470561..8b644ea39725 100644
> --- a/tools/testing/selftests/cgroup/cgroup_util.c
> +++ b/tools/testing/selftests/cgroup/cgroup_util.c
> @@ -89,17 +89,28 @@ int cg_read(const char *cgroup, const char *control, char
> *buf, size_t len)
> int cg_read_strcmp(const char *cgroup, const char *control,
> const char *expected)
> {
> - size_t size = strlen(expected) + 1;
> + size_t size;
> char *buf;
> + int ret;
> +
> + /* Handle the case of comparing against empty string */
> + if (!expected)
> + size = 32;
This doesn't look right. I would think expected shouldn't be null?
It gets used below.
> + else
> + size = strlen(expected) + 1;
>
> buf = malloc(size);
> if (!buf)
> return -1;
>
> - if (cg_read(cgroup, control, buf, size))
> + if (cg_read(cgroup, control, buf, size)) {
> + free(buf);
> return -1;
> + }
>
> - return strcmp(expected, buf);
> + ret = strcmp(expected, buf);
If expected is null, what's the point in running the test?
Is empty "needle" string a valid test scenario?
> + free(buf);
> + return ret;
> }
>
> int cg_read_strstr(const char *cgroup, const char *control, const char
> *needle)
>
thanks,
-- Shuah
Re: [PATCH 1/2] Fix cg_read_strcmp()
On Fri, Sep 07, 2018 at 09:49:23AM -0700, jgka...@fb.com wrote:
> From: Jay Kamat
>
> Fix a couple issues with cg_read_strcmp(), to improve correctness of
> cgroup tests
> - Fix cg_read_strcmp() always returning 0 for empty "needle" strings
> - Fix a memory leak in cg_read_strcmp()
>
> Fixes: 84092dbcf901 ("selftests: cgroup: add memory controller self-tests")
Hi Jay!
Thank you for working on this!
Acked-by: Roman Gushchin
Re: [PATCH 1/2] Fix cg_read_strcmp()
On Fri, Sep 07, 2018 at 09:49:23AM -0700, jgka...@fb.com wrote:
> From: Jay Kamat
>
> Fix a couple issues with cg_read_strcmp(), to improve correctness of
> cgroup tests
> - Fix cg_read_strcmp() always returning 0 for empty "needle" strings
> - Fix a memory leak in cg_read_strcmp()
>
> Fixes: 84092dbcf901 ("selftests: cgroup: add memory controller self-tests")
Hi Jay!
Thank you for working on this!
Acked-by: Roman Gushchin
[PATCH 1/2] Fix cg_read_strcmp()
From: Jay Kamat
Fix a couple issues with cg_read_strcmp(), to improve correctness of
cgroup tests
- Fix cg_read_strcmp() always returning 0 for empty "needle" strings
- Fix a memory leak in cg_read_strcmp()
Fixes: 84092dbcf901 ("selftests: cgroup: add memory controller self-tests")
Signed-off-by: Jay Kamat
---
tools/testing/selftests/cgroup/cgroup_util.c | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/cgroup/cgroup_util.c
b/tools/testing/selftests/cgroup/cgroup_util.c
index 1e9e3c470561..8b644ea39725 100644
--- a/tools/testing/selftests/cgroup/cgroup_util.c
+++ b/tools/testing/selftests/cgroup/cgroup_util.c
@@ -89,17 +89,28 @@ int cg_read(const char *cgroup, const char *control, char
*buf, size_t len)
int cg_read_strcmp(const char *cgroup, const char *control,
const char *expected)
{
- size_t size = strlen(expected) + 1;
+ size_t size;
char *buf;
+ int ret;
+
+ /* Handle the case of comparing against empty string */
+ if (!expected)
+ size = 32;
+ else
+ size = strlen(expected) + 1;
buf = malloc(size);
if (!buf)
return -1;
- if (cg_read(cgroup, control, buf, size))
+ if (cg_read(cgroup, control, buf, size)) {
+ free(buf);
return -1;
+ }
- return strcmp(expected, buf);
+ ret = strcmp(expected, buf);
+ free(buf);
+ return ret;
}
int cg_read_strstr(const char *cgroup, const char *control, const char *needle)
--
2.17.1
[PATCH 1/2] Fix cg_read_strcmp()
From: Jay Kamat
Fix a couple issues with cg_read_strcmp(), to improve correctness of
cgroup tests
- Fix cg_read_strcmp() always returning 0 for empty "needle" strings
- Fix a memory leak in cg_read_strcmp()
Fixes: 84092dbcf901 ("selftests: cgroup: add memory controller self-tests")
Signed-off-by: Jay Kamat
---
tools/testing/selftests/cgroup/cgroup_util.c | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/cgroup/cgroup_util.c
b/tools/testing/selftests/cgroup/cgroup_util.c
index 1e9e3c470561..8b644ea39725 100644
--- a/tools/testing/selftests/cgroup/cgroup_util.c
+++ b/tools/testing/selftests/cgroup/cgroup_util.c
@@ -89,17 +89,28 @@ int cg_read(const char *cgroup, const char *control, char
*buf, size_t len)
int cg_read_strcmp(const char *cgroup, const char *control,
const char *expected)
{
- size_t size = strlen(expected) + 1;
+ size_t size;
char *buf;
+ int ret;
+
+ /* Handle the case of comparing against empty string */
+ if (!expected)
+ size = 32;
+ else
+ size = strlen(expected) + 1;
buf = malloc(size);
if (!buf)
return -1;
- if (cg_read(cgroup, control, buf, size))
+ if (cg_read(cgroup, control, buf, size)) {
+ free(buf);
return -1;
+ }
- return strcmp(expected, buf);
+ ret = strcmp(expected, buf);
+ free(buf);
+ return ret;
}
int cg_read_strstr(const char *cgroup, const char *control, const char *needle)
--
2.17.1
Re: [PATCH 1/2] Fix cg_read_strcmp()
Hi Jay,
Thanks for the patch. Couple of comments below.
On 09/04/2018 07:08 PM, jgka...@fb.com wrote:
> From: Jay Kamat
>
> Fix a couple issues with cg_read_strcmp(), to improve correctness of
> cgroup tests
> - Fix cg_read_strcmp() always returning 0 for empty "needle" strings
> - Fix a memory leak in cg_read_strcmp()
>
> Fixes: 84092dbcf901 ("selftests: cgroup: add memory controller self-tests")
>
> Signed-off-by: Jay Kamat
> ---
> tools/testing/selftests/cgroup/cgroup_util.c | 13 +++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/cgroup/cgroup_util.c
> b/tools/testing/selftests/cgroup/cgroup_util.c
> index 1e9e3c470561..4aadf38bcd5d 100644
> --- a/tools/testing/selftests/cgroup/cgroup_util.c
> +++ b/tools/testing/selftests/cgroup/cgroup_util.c
> @@ -91,15 +91,24 @@ int cg_read_strcmp(const char *cgroup, const char
> *control,
> {
> size_t size = strlen(expected) + 1;
> char *buf;
> + int ret;
> +
> + /* Handle the case of comparing against empty string */
> + if (size == 1)
> + size = 32;
Why not test for !expected and avoid strlen(expected) all together?
>
> buf = malloc(size);
> if (!buf)
> return -1;
>
> - if (cg_read(cgroup, control, buf, size))
> + if (cg_read(cgroup, control, buf, size)) {
> + free(buf);
> return -1;
> + }
>
> - return strcmp(expected, buf);
> + ret = strcmp(expected, buf);
> + free(buf);
> + return ret;
> }
>
> int cg_read_strstr(const char *cgroup, const char *control, const char
> *needle)
>
thanks,
-- Shuah
Re: [PATCH 1/2] Fix cg_read_strcmp()
Hi Jay,
Thanks for the patch. Couple of comments below.
On 09/04/2018 07:08 PM, jgka...@fb.com wrote:
> From: Jay Kamat
>
> Fix a couple issues with cg_read_strcmp(), to improve correctness of
> cgroup tests
> - Fix cg_read_strcmp() always returning 0 for empty "needle" strings
> - Fix a memory leak in cg_read_strcmp()
>
> Fixes: 84092dbcf901 ("selftests: cgroup: add memory controller self-tests")
>
> Signed-off-by: Jay Kamat
> ---
> tools/testing/selftests/cgroup/cgroup_util.c | 13 +++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/cgroup/cgroup_util.c
> b/tools/testing/selftests/cgroup/cgroup_util.c
> index 1e9e3c470561..4aadf38bcd5d 100644
> --- a/tools/testing/selftests/cgroup/cgroup_util.c
> +++ b/tools/testing/selftests/cgroup/cgroup_util.c
> @@ -91,15 +91,24 @@ int cg_read_strcmp(const char *cgroup, const char
> *control,
> {
> size_t size = strlen(expected) + 1;
> char *buf;
> + int ret;
> +
> + /* Handle the case of comparing against empty string */
> + if (size == 1)
> + size = 32;
Why not test for !expected and avoid strlen(expected) all together?
>
> buf = malloc(size);
> if (!buf)
> return -1;
>
> - if (cg_read(cgroup, control, buf, size))
> + if (cg_read(cgroup, control, buf, size)) {
> + free(buf);
> return -1;
> + }
>
> - return strcmp(expected, buf);
> + ret = strcmp(expected, buf);
> + free(buf);
> + return ret;
> }
>
> int cg_read_strstr(const char *cgroup, const char *control, const char
> *needle)
>
thanks,
-- Shuah
[PATCH 1/2] Fix cg_read_strcmp()
From: Jay Kamat
Fix a couple issues with cg_read_strcmp(), to improve correctness of
cgroup tests
- Fix cg_read_strcmp() always returning 0 for empty "needle" strings
- Fix a memory leak in cg_read_strcmp()
Fixes: 84092dbcf901 ("selftests: cgroup: add memory controller self-tests")
Signed-off-by: Jay Kamat
---
tools/testing/selftests/cgroup/cgroup_util.c | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/cgroup/cgroup_util.c
b/tools/testing/selftests/cgroup/cgroup_util.c
index 1e9e3c470561..4aadf38bcd5d 100644
--- a/tools/testing/selftests/cgroup/cgroup_util.c
+++ b/tools/testing/selftests/cgroup/cgroup_util.c
@@ -91,15 +91,24 @@ int cg_read_strcmp(const char *cgroup, const char *control,
{
size_t size = strlen(expected) + 1;
char *buf;
+ int ret;
+
+ /* Handle the case of comparing against empty string */
+ if (size == 1)
+ size = 32;
buf = malloc(size);
if (!buf)
return -1;
- if (cg_read(cgroup, control, buf, size))
+ if (cg_read(cgroup, control, buf, size)) {
+ free(buf);
return -1;
+ }
- return strcmp(expected, buf);
+ ret = strcmp(expected, buf);
+ free(buf);
+ return ret;
}
int cg_read_strstr(const char *cgroup, const char *control, const char *needle)
--
2.17.1
[PATCH 1/2] Fix cg_read_strcmp()
From: Jay Kamat
Fix a couple issues with cg_read_strcmp(), to improve correctness of
cgroup tests
- Fix cg_read_strcmp() always returning 0 for empty "needle" strings
- Fix a memory leak in cg_read_strcmp()
Fixes: 84092dbcf901 ("selftests: cgroup: add memory controller self-tests")
Signed-off-by: Jay Kamat
---
tools/testing/selftests/cgroup/cgroup_util.c | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/cgroup/cgroup_util.c
b/tools/testing/selftests/cgroup/cgroup_util.c
index 1e9e3c470561..4aadf38bcd5d 100644
--- a/tools/testing/selftests/cgroup/cgroup_util.c
+++ b/tools/testing/selftests/cgroup/cgroup_util.c
@@ -91,15 +91,24 @@ int cg_read_strcmp(const char *cgroup, const char *control,
{
size_t size = strlen(expected) + 1;
char *buf;
+ int ret;
+
+ /* Handle the case of comparing against empty string */
+ if (size == 1)
+ size = 32;
buf = malloc(size);
if (!buf)
return -1;
- if (cg_read(cgroup, control, buf, size))
+ if (cg_read(cgroup, control, buf, size)) {
+ free(buf);
return -1;
+ }
- return strcmp(expected, buf);
+ ret = strcmp(expected, buf);
+ free(buf);
+ return ret;
}
int cg_read_strstr(const char *cgroup, const char *control, const char *needle)
--
2.17.1
