Re: [PATCH 1/2] v4l-helpers: Don't close the fd in {}_s_fd
Hey Hans,
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.
>
This patch is also needed to prevent the compliance tool
from unintendenly closing a descriptor.
diff --git a/utils/common/v4l-helpers.h b/utils/common/v4l-helpers.h
index 27683a3d286d..45ed997379a1 100644
--- a/utils/common/v4l-helpers.h
+++ b/utils/common/v4l-helpers.h
@@ -118,7 +118,11 @@ static inline int v4l_wrap_open(struct v4l_fd *f,
const char *file, int oflag, .
static inline int v4l_wrap_close(struct v4l_fd *f)
{
- return close(f->fd);
+ int ret;
+
+ ret = close(f->fd);
+ f->fd = -1;
+ return ret;
}
static inline ssize_t v4l_wrap_read(struct v4l_fd *f, void *buffer,
size_t n)
Regards,
Eze
Re: [PATCH 1/2] v4l-helpers: Don't close the fd in {}_s_fd
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.
Re: [PATCH 1/2] v4l-helpers: Don't close the fd in {}_s_fd
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
> ---
> 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)
>
[PATCH 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
---
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)
--
2.18.0.rc2
