<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <p>Hi,<br>
    </p>
    <br>
    <div class="moz-cite-prefix">On 8/2/2018 4:13 PM, Maarten Lankhorst
      wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:8a80db90-7b62-0242-37a8-d46515bd024a@linux.intel.com">
      <pre wrap="">Op 02-08-18 om 12:42 schreef Maarten Lankhorst:
</pre>
      <blockquote type="cite">
        <pre wrap="">Op 02-07-18 om 13:27 schreef Maarten Lankhorst:
</pre>
        <blockquote type="cite">
          <pre wrap="">Op 02-07-18 om 13:16 schreef Mahesh Kumar:
</pre>
          <blockquote type="cite">
            <pre wrap="">Now crc-core framework verifies the source string passed by the user.
So setting bad-source will fail. Expect file write to fail in bad-source
subtest of kms_pipe_crc_basic.

Signed-off-by: Mahesh Kumar <a class="moz-txt-link-rfc2396E" href="mailto:mahesh1.kumar@intel.com"><mahesh1.kumar@intel.com></a>
---
 tests/kms_pipe_crc_basic.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c
index 235fdc38..2d4dfee8 100644
--- a/tests/kms_pipe_crc_basic.c
+++ b/tests/kms_pipe_crc_basic.c
@@ -48,8 +48,7 @@ static struct {
 
 static void test_bad_source(data_t *data)
 {
-       igt_assert(igt_sysfs_set(data->debugfs, "crtc-0/crc/control", "foo"));
-       igt_assert(openat(data->debugfs, "crtc-0/crc/data", O_WRONLY) == -1);
+       igt_assert(!igt_sysfs_set(data->debugfs, "crtc-0/crc/control", "foo"));
 }
 
 #define N_CRCS 3
</pre>
          </blockquote>
          <pre wrap="">New behavior makes more sense.

Reviewed-by: Maarten Lankhorst <a class="moz-txt-link-rfc2396E" href="mailto:maarten.lankhorst@linux.intel.com"><maarten.lankhorst@linux.intel.com></a>

Do you have igt commit rights?

</pre>
        </blockquote>
        <pre wrap="">Any objections if we change this to allow both behaviors?

diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c
index 235fdc386ba2..91909fa42f2f 100644
--- a/tests/kms_pipe_crc_basic.c
+++ b/tests/kms_pipe_crc_basic.c
@@ -48,8 +48,11 @@ static struct {
 
 static void test_bad_source(data_t *data)
 {
-       igt_assert(igt_sysfs_set(data->debugfs, "crtc-0/crc/control", "foo"));
-       igt_assert(openat(data->debugfs, "crtc-0/crc/data", O_WRONLY) == -1);
+       errno = 0;
+       if (igt_sysfs_set(data->debugfs, "crtc-0/crc/control", "foo"))
+               igt_assert(openat(data->debugfs, "crtc-0/crc/data", O_WRONLY) == -1);
+       else
+               igt_assert_eq(errno, EINVAL);
 }
 
 #define N_CRCS 3

</pre>
      </blockquote>
      <pre wrap="">Hm without the else, errno should be EINVAL in any case..</pre>
    </blockquote>
    agree, with this change<br>
    Reviewed-by: Mahesh Kumar <a class="moz-txt-link-rfc2396E"
      href="mailto:mahesh1.kumar@intel.com"><mahesh1.kumar@intel.com>
    </a>
    <blockquote type="cite"
      cite="mid:8a80db90-7b62-0242-37a8-d46515bd024a@linux.intel.com">
      <pre wrap="">
</pre>
    </blockquote>
    <br>
  </body>
</html>