[igt-dev] [PATCH i-g-t] lib/ioctl_wrappers: Assert that prime_handle_to_fd returns a valid fd

[Chris Wilson]

Quoting Petri Latvala (2019-03-12 13:00:04)
> On Tue, Mar 12, 2019 at 11:34:37AM +0000, Chris Wilson wrote:
> > Quoting Petri Latvala (2019-03-12 11:21:14)
> > > If the ioctl is successful, the returned fd should be valid. Check
> > > that it is, thus also helping static analysis in almost 70 call sites.
> > > 
> > > Signed-off-by: Petri Latvala <petri.latvala at intel.com>
> > > ---
> > >  lib/ioctl_wrappers.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> > > index 39920f87..31969e77 100644
> > > --- a/lib/ioctl_wrappers.c
> > > +++ b/lib/ioctl_wrappers.c
> > > @@ -1332,6 +1332,7 @@ int prime_handle_to_fd(int fd, uint32_t handle)
> > >         args.fd = -1;
> > >  
> > >         do_ioctl(fd, DRM_IOCTL_PRIME_HANDLE_TO_FD, &args);
> > > +       igt_assert_fd(args.fd);
> > 
> > Why? Why would we be using a static analysis tool that complains about
> > feeding invalid data to the kernel, because that is exactly the goal of
> > igt.
> 
> Because after teaching it what data should be invalid and what should
> be valid, it has already revealed a couple of bugs?

Did the kernel handle the invalid fd correctly? That is what is
important. It's not a bug for igt to call close(-1) or whatever.
 
> Are you objecting to checking that the ioctl gives an fd that is >= 0?
> Where's the line here, what parts of the uapi semantics must always be
> left unchecked in a test suite for uapi?

No, my opinion is htat this assertion is only valid in a test not library
code, as the library code should just be a conduit and even the do_ioctl
is a mistake as there is no higher level wrapper here.
-Chris