Re: [PATCH igt 4/9] igt/vc4_wait_bo: Add a test for VC4's wait-for-BO ioctl.

From: Daniel Stone <daniel@fooishbar.org>
To: Eric Anholt <eric@anholt.net>
Cc: intel-gfx <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH igt 4/9] igt/vc4_wait_bo: Add a test for VC4's wait-for-BO ioctl.
Date: Thu, 4 Feb 2016 15:22:33 +0000	[thread overview]
Message-ID: <CAPj87rOTXhUbvb5aK9X6ghnfLhAs1q8wfrBcOQBFz-Q-G5Yq4g@mail.gmail.com> (raw)
In-Reply-To: <1454535703-660-4-git-send-email-eric@anholt.net>

Hi,

On 3 February 2016 at 21:41, Eric Anholt <eric@anholt.net> wrote:
> +               ret = ioctl(fd, DRM_IOCTL_VC4_WAIT_BO, &arg);
> +               igt_assert(ret == -1 && errno == EINVAL);

A couple of nitpicks: all these should be either do_ioctl() for
success, or do_ioctl_err() for failure, which not only cuts down the
number of lines a bit, but also shows you the exact condition which
occurred (e.g. it'll show that errno was -EBUSY rather than expected
-EINVAL without having to round-trip through the reporter). Similarly,
all your igt_assert(x == y) should be igt_assert_eq(x, y), or any of
the igt_assert_{eq,neq} variants, e.g. _u32 for comparing uint32_t.
You can also use do_or_die(foo) as shorthand for igt_assert_eq(foo,
0).

With those addressed:
Reviewed-by: Daniel Stone <daniels@collabora.com>

Cheers,
Daniel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx