On Mon, Jan 13, 2025 at 12:10:43AM +0100, Kirill A. Korinsky wrote:
> On Sun, 12 Jan 2025 21:45:14 +0100,
> Jeremie Courreges-Anglas <jca@wxcvbn.org> wrote:
> >
> > I was really talking about additional fields in (pre)existing structs.
> > See:
> >
> > /* capabilities for struct v4l2_requestbuffers and v4l2_create_buffers */
> >
> > I think the capabilities field in struct v4l2_requestbuffers should be
> > set (or zeroed?) by uvideo/utvfu.  Else an application that starts
> > picking up this field might see uninitialized data if it doesn't zero
> > the field before calling the ioctl.  I'm not seeing similar fields in
> > other preexisting structs (full list: v4l2_fmtdesc,
> > v4l2_requestbuffers, v4l2_buffer, v4l2_ext_controls;
> > v4l2_create_buffers is modified but VIDIOC_CREATE_BUFS isn't
> > implemented).
> >
> > > > Just sharing my thoughts, I know next to nothing about the use of V4L2
> > > > API and its use in the ecosystem.
> > > >
> > >
> > > Thanks, I appriciete it, and this is quite large diff...
> > >
> > > > Regarding the content of the diff itself, the #ifdef/#ifndef
> > > > __KERNEL__ chunks look wrong, the define we use is _KERNEL.  The
> > > > #ifdef ones should probably be dropped if we don't plan to use the
> > > > definitions they're hiding in our kernel.  For example struct
> > > > __kernel_v4l2_timeval (ugh) looks out of place on OpenBSD.  The
> > > > #ifndef chunks should probably be adapted to our #ifndef _KERNEL
> > > > idiom.
> > > >
> > >
> > > The currect code contains 3 __KERNEL__ and 1 _KERNEL. My diff adds a few
> > > more, and replaces _KERNEL to __KERNEL__. I not sure that it is bad things,
> > > special if such macros is wrong. Yes, I may clean up this code, but it
> > > increases burden for future sync.
> > >
> > > Not sure that it is a good idea.
> >
> > I had not noticed the existing occurrences of #ifndef __KERNEL__.  The
> > actual name checked doesn't matter much technically since it's not
> > supposed to be defined when used from userland.  It looks weird and
> > inconsistent with the rest of our includes, but I don't feel too
> > strongly about those #ifndef.
> >
> > The #ifdef __KERNEL__ bits are really ugly though.  It shouldn't be
> > too hard to skip them when diffing against the linux header.
> >
> 
> Thanks for both points. I absolutley agree with you and here updated diff
> where I removed __KERNEL__, and fixed uvideo and utvfu.

Heh, so you completely dropped the #ifndef __KERNEL__ bits.  I guess
that works too.  LGTM fwiw.  I'll keep you updated with the bulk build
results when I get them.

-- 
jca