[1/2] v4l-helpers: Don't close the fd in {}_s_fd

When creating a second node via copy or assignment:

    node2 = node

The node being assigned to, i.e. node2, obtains the fd.
This causes a later call to node2.media_open to close()
the fd, thus unintendenly closing the original node fd,
via the call path (e.g. for media devices):

  node2.media_open
     v4l_media_open
        v4l_media_s_fd

Similar call paths apply for other device types.
Fix this by removing the close in xxx_s_fd.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 utils/common/v4l-helpers.h | 9 ---------
 1 file changed, 9 deletions(-)
  

On 06/28/2018 09:25 PM, Ezequiel Garcia wrote:
> When creating a second node via copy or assignment:
> 
>     node2 = node
> 
> The node being assigned to, i.e. node2, obtains the fd.
> This causes a later call to node2.media_open to close()
> the fd, thus unintendenly closing the original node fd,
> via the call path (e.g. for media devices):
> 
>   node2.media_open
>      v4l_media_open
>         v4l_media_s_fd
> 
> Similar call paths apply for other device types.
> Fix this by removing the close in xxx_s_fd.

I fixed this in a different way by overloading the assignment operator
and calling dup(fd). That solves this as well.

Regards,

	Hans

> 
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
>  utils/common/v4l-helpers.h | 9 ---------
>  1 file changed, 9 deletions(-)
> 
> diff --git a/utils/common/v4l-helpers.h b/utils/common/v4l-helpers.h
> index c37b72712126..83d8d7d9c073 100644
> --- a/utils/common/v4l-helpers.h
> +++ b/utils/common/v4l-helpers.h
> @@ -444,9 +444,6 @@ static inline int v4l_s_fd(struct v4l_fd *f, int fd, const char *devname, bool d
>  	struct v4l2_queryctrl qc;
>  	struct v4l2_selection sel;
>  
> -	if (f->fd >= 0)
> -		f->close(f);
> -
>  	f->fd = fd;
>  	f->direct = direct;
>  	if (fd < 0)
> @@ -492,9 +489,6 @@ static inline int v4l_open(struct v4l_fd *f, const char *devname, bool non_block
>  
>  static inline int v4l_subdev_s_fd(struct v4l_fd *f, int fd, const char *devname)
>  {
> -	if (f->fd >= 0)
> -		f->close(f);
> -
>  	f->fd = fd;
>  	f->direct = false;
>  	if (fd < 0)
> @@ -525,9 +519,6 @@ static inline int v4l_subdev_open(struct v4l_fd *f, const char *devname, bool no
>  
>  static inline int v4l_media_s_fd(struct v4l_fd *f, int fd, const char *devname)
>  {
> -	if (f->fd >= 0)
> -		f->close(f);
> -
>  	f->fd = fd;
>  	f->direct = false;
>  	if (fd < 0)
>
  

On Fri, 2018-06-29 at 09:03 +0200, Hans Verkuil wrote:
> On 06/28/2018 09:25 PM, Ezequiel Garcia wrote:
> > When creating a second node via copy or assignment:
> > 
> >     node2 = node
> > 
> > The node being assigned to, i.e. node2, obtains the fd.
> > This causes a later call to node2.media_open to close()
> > the fd, thus unintendenly closing the original node fd,
> > via the call path (e.g. for media devices):
> > 
> >   node2.media_open
> >      v4l_media_open
> >         v4l_media_s_fd
> > 
> > Similar call paths apply for other device types.
> > Fix this by removing the close in xxx_s_fd.
> 
> I fixed this in a different way by overloading the assignment
> operator
> and calling dup(fd). That solves this as well.
> 

Yes, but I am now seeing another EBADF error in the compliance run.

close(3)                                = 0
openat(AT_FDCWD, "/dev/video2", O_RDWR) = 3
close(3)                                = 0
ioctl(3, VIDIOC_QUERYCAP, 0x7ffe54788794) = -1 EBADF
close(3)                                = -1 EBADF

Let me see if I can dig it.