[igt-dev] [PATCH i-g-t 2/2] tests/kms_addfb_basic: Relax errno assertions

[Petri Latvala]

On Fri, Nov 13, 2020 at 04:41:16PM -0500, Jeremy Cline wrote:
> On Fri, Nov 13, 2020 at 01:59:55PM -0500, Lyude Paul wrote:
> > On Fri, 2020-11-13 at 11:49 -0500, Jeremy Cline wrote:
> > > This adjusts the check on failed calls to DRM_IOCTL_MODE_ADDFB2 in
> > > various scenarios. According to the kernel's
> > > drm_mode_config_funcs.fb_create documentation, a negative return code is
> > > all that is required to signal failure, and different drivers return
> > > different errors in these scenarios.
> > > 
> > > Particularly, Nouveau returns -ERANGE rather than -EINVAL in scenarios
> > > where the buffer is too small for the proposed configuration.
> > > 
> > > Signed-off-by: Jeremy Cline <jcline at redhat.com>
> > > ---
> > >  tests/kms_addfb_basic.c | 16 ++++++++--------
> > >  1 file changed, 8 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/tests/kms_addfb_basic.c b/tests/kms_addfb_basic.c
> > > index 0ec583fe..b284bfc1 100644
> > > --- a/tests/kms_addfb_basic.c
> > > +++ b/tests/kms_addfb_basic.c
> > > @@ -282,7 +282,7 @@ static void pitch_tests(int fd)
> > >                 igt_subtest_f("bad-pitch-%i", bad_pitches[i]) {
> > >                         f.pitches[0] = bad_pitches[i];
> > >                         igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, &f) ==
> > > -1 &&
> > > -                                  errno == EINVAL);
> > > +                                  errno != 0);
> > >                 }
> > >         }
> > >  
> > > @@ -413,11 +413,11 @@ static void size_tests(int fd)
> > >         f_8.width++;
> > >         igt_subtest("too-wide") {
> > >                 igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, &f) == -1 &&
> > > -                          errno == EINVAL);
> > > +                          errno != 0);
> > >                 igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, &f_16) == -1 &&
> > > -                          errno == EINVAL);
> > > +                          errno != 0);
> > >                 igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, &f_8) == -1 &&
> > > -                          errno == EINVAL);
> > > +                          errno != 0);
> > >         }
> > >         f.width--;
> > >         f_16.width--;
> > > @@ -427,17 +427,17 @@ static void size_tests(int fd)
> > >         f_8.height++;
> > >         igt_subtest("too-high") {
> > >                 igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, &f) == -1 &&
> > > -                          errno == EINVAL);
> > > +                          errno != 0);
> > >                 igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, &f_16) == -1 &&
> > > -                          errno == EINVAL);
> > > +                          errno != 0);
> > >                 igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, &f_8) == -1 &&
> > > -                          errno == EINVAL);
> > > +                          errno != 0);
> > >         }
> > >  
> > >         f.handles[0] = gem_bo_small;
> > >         igt_subtest("bo-too-small") {
> > >                 igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, &f) == -1 &&
> > > -                          errno == EINVAL);
> > > +                          errno != 0);
> > 
> > Probably should be do_ioctl_err() instead. JFYI too, there's an old outdated
> > cocci file for converting things like this automatically that I meant to update.
> > I made some progress, but had to put it on the backburner because of time
> > constraints.
> 
> Ah, neat, thanks for the pointer. It looks like the macro wants one to
> provide the exact error. It seems like breaking this out into
> vendor-specific tests where it checks for EINVAL for Intel and ERANGE
> for Nouveau would be better in any case. Assuming I do that, would it be
> good to leave this here for generic "at least don't succeed" testing for
> any additional vendors?

Yeah, +1 for vendor-specific error code checks, +1 for fallback to
"any error". Also add an igt_info() in the fallback that tells what
the error code was.


-- 
Petri Latvala


> 
> - Jeremy
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev