* [LTP] [PATCH 1/1] do not test undefined behavior
@ 2017-06-01 15:16 Martin Karac
2017-06-02 11:45 ` Jan Stancek
0 siblings, 1 reply; 5+ messages in thread
From: Martin Karac @ 2017-06-01 15:16 UTC (permalink / raw)
To: ltp
The test_flags() function's first test for each file is an attempt
to write a space character into that file (NULL translates to " ").
The test expects that this attempt will be successful and that the flag
will get set to 0.
This behavior was changed in Linux kernel between versions 3.13.74 and
3.14. with the commit a742c59de66ea080afa3edaf3428b3cdd5aa87cd
"cgroup: unify cgroup_write_X64() and cgroup_write_string()".
With the new behavior, attempting to write a space character into
a flag file returns EINVAL; I find this behavior more consistent.
Flag files are an interface which is known to expect numeric values.
We already have a test in test_flags() which covers invalid input.
We should not attempt to write a space into a flag file because
the resulting behavior is not strictly defined anywhere.
Therefore, it would be best to drop the first test.
---
.../cpuset_base_ops_testset.sh | 1 -
1 file changed, 1 deletion(-)
diff --git a/testcases/kernel/controllers/cpuset/cpuset_base_ops_test/cpuset_base_ops_testset.sh b/testcases/kernel/controllers/cpuset/cpuset_base_ops_test/cpuset_base_ops_testset.sh
index 992b8f2..70e7203 100755
--- a/testcases/kernel/controllers/cpuset/cpuset_base_ops_test/cpuset_base_ops_testset.sh
+++ b/testcases/kernel/controllers/cpuset/cpuset_base_ops_test/cpuset_base_ops_testset.sh
@@ -181,7 +181,6 @@ test_flags()
do
base_op_test "$CPUSET/$filename" "$flags" "$result"
done <<- EOF
- NULL 0
0 0
1 1
-1 WRITE_ERROR
--
1.7.9.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [LTP] [PATCH 1/1] do not test undefined behavior
2017-06-01 15:16 [LTP] [PATCH 1/1] do not test undefined behavior Martin Karac
@ 2017-06-02 11:45 ` Jan Stancek
2017-06-02 12:11 ` Martin Karac
0 siblings, 1 reply; 5+ messages in thread
From: Jan Stancek @ 2017-06-02 11:45 UTC (permalink / raw)
To: ltp
----- Original Message -----
> The test_flags() function's first test for each file is an attempt
> to write a space character into that file (NULL translates to " ").
> The test expects that this attempt will be successful and that the flag
> will get set to 0.
>
> This behavior was changed in Linux kernel between versions 3.13.74 and
> 3.14. with the commit a742c59de66ea080afa3edaf3428b3cdd5aa87cd
> "cgroup: unify cgroup_write_X64() and cgroup_write_string()".
> With the new behavior, attempting to write a space character into
> a flag file returns EINVAL; I find this behavior more consistent.
>
> Flag files are an interface which is known to expect numeric values.
> We already have a test in test_flags() which covers invalid input.
> We should not attempt to write a space into a flag file because
> the resulting behavior is not strictly defined anywhere.
> Therefore, it would be best to drop the first test.
Hi,
no objections to change you are proposing, just comments on format:
You're missing Signed-off-by line.
Patch appears to be mangled, all tabs were replaced by spaces.
Did you send the patch with git send-email? Would you care to
try resend?
Regards,
Jan
^ permalink raw reply [flat|nested] 5+ messages in thread
* [LTP] [PATCH 1/1] do not test undefined behavior
2017-06-02 11:45 ` Jan Stancek
@ 2017-06-02 12:11 ` Martin Karac
0 siblings, 0 replies; 5+ messages in thread
From: Martin Karac @ 2017-06-02 12:11 UTC (permalink / raw)
To: ltp
Hi Jan,
thank you very much for your comments. This is my first ever attempt to send a patch.
I did not send it with git send-email, but I will definitely try.
With regards,
Martin Karac
On 06/ 2/17 01:45 PM, Jan Stancek wrote:
>
>
> ----- Original Message -----
>> The test_flags() function's first test for each file is an attempt
>> to write a space character into that file (NULL translates to " ").
>> The test expects that this attempt will be successful and that the flag
>> will get set to 0.
>>
>> This behavior was changed in Linux kernel between versions 3.13.74 and
>> 3.14. with the commit a742c59de66ea080afa3edaf3428b3cdd5aa87cd
>> "cgroup: unify cgroup_write_X64() and cgroup_write_string()".
>> With the new behavior, attempting to write a space character into
>> a flag file returns EINVAL; I find this behavior more consistent.
>>
>> Flag files are an interface which is known to expect numeric values.
>> We already have a test in test_flags() which covers invalid input.
>> We should not attempt to write a space into a flag file because
>> the resulting behavior is not strictly defined anywhere.
>> Therefore, it would be best to drop the first test.
>
> Hi,
>
> no objections to change you are proposing, just comments on format:
>
> You're missing Signed-off-by line.
> Patch appears to be mangled, all tabs were replaced by spaces.
>
> Did you send the patch with git send-email? Would you care to
> try resend?
>
> Regards,
> Jan
>
--
Martin Karac
PLQA - Prague Linux QA
Oracle Czech s.r.o.
Prague, Czech Republic
^ permalink raw reply [flat|nested] 5+ messages in thread
* [LTP] [PATCH 1/1] do not test undefined behavior
@ 2017-06-05 17:00 Martin Karac
2017-06-07 10:50 ` Jan Stancek
0 siblings, 1 reply; 5+ messages in thread
From: Martin Karac @ 2017-06-05 17:00 UTC (permalink / raw)
To: ltp
The test_flags() function's first test for each file is an attempt
to write a space character into that file (NULL translates to " ").
The test expects that this attempt will be successful and that the flag
will get set to 0.
This behavior was changed in Linux kernel between versions 3.13.74 and
3.14. with the commit a742c59de66ea080afa3edaf3428b3cdd5aa87cd
"cgroup: unify cgroup_write_X64() and cgroup_write_string()".
With the new behavior, attempting to write a space character into
a flag file returns EINVAL; I find this behavior more consistent.
Flag files are an interface which is known to expect numeric values.
We already have a test in test_flags() which covers invalid input.
We should not attempt to write a space into a flag file because
the resulting behavior is not strictly defined anywhere.
Therefore, it would be best to drop the first test.
Signed-off-by: Martin Karac <martin.karac@oracle.com>
---
.../cpuset_base_ops_testset.sh | 1 -
1 file changed, 1 deletion(-)
diff --git a/testcases/kernel/controllers/cpuset/cpuset_base_ops_test/cpuset_base_ops_testset.sh b/testcases/kernel/controllers/cpuset/cpuset_base_ops_test/cpuset_base_ops_testset.sh
index 992b8f2..70e7203 100755
--- a/testcases/kernel/controllers/cpuset/cpuset_base_ops_test/cpuset_base_ops_testset.sh
+++ b/testcases/kernel/controllers/cpuset/cpuset_base_ops_test/cpuset_base_ops_testset.sh
@@ -181,7 +181,6 @@ test_flags()
do
base_op_test "$CPUSET/$filename" "$flags" "$result"
done <<- EOF
- NULL 0
0 0
1 1
-1 WRITE_ERROR
--
1.7.9.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [LTP] [PATCH 1/1] do not test undefined behavior
2017-06-05 17:00 Martin Karac
@ 2017-06-07 10:50 ` Jan Stancek
0 siblings, 0 replies; 5+ messages in thread
From: Jan Stancek @ 2017-06-07 10:50 UTC (permalink / raw)
To: ltp
----- Original Message -----
> Flag files are an interface which is known to expect numeric values.
> We already have a test in test_flags() which covers invalid input.
> We should not attempt to write a space into a flag file because
> the resulting behavior is not strictly defined anywhere.
> Therefore, it would be best to drop the first test.
Pushed.
Thanks,
Jan
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-06-07 10:50 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-06-01 15:16 [LTP] [PATCH 1/1] do not test undefined behavior Martin Karac
2017-06-02 11:45 ` Jan Stancek
2017-06-02 12:11 ` Martin Karac
-- strict thread matches above, loose matches on Subject: below --
2017-06-05 17:00 Martin Karac
2017-06-07 10:50 ` Jan Stancek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).