linux-kernel - Re: [PATCH 1/6] media: v4l2-subdev: stub v4l2_subdev_get_try

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20181209211756.kptepm4a6aufa6vf@valkosipuli.retiisi.org.uk>
Date:   Sun, 9 Dec 2018 23:17:56 +0200
From:   sakari.ailus@....fi
To:     Lubomir Rintel <lkundrak@...sk>
Cc:     jacopo mondi <jacopo@...ndi.org>,
        "Lad, Prabhakar" <prabhakar.csengg@...il.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Rui Miguel Silva <rmfrfs@...il.com>,
        Shunqian Zheng <zhengsq@...k-chips.com>,
        Jonathan Corbet <corbet@....net>,
        Wenyou Yang <wenyou.yang@...rochip.com>,
        linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/6] media: v4l2-subdev: stub
 v4l2_subdev_get_try_format() ??

Hi Lubomir, Jacopo,

On Tue, Dec 04, 2018 at 04:01:43PM +0100, Lubomir Rintel wrote:
> On Mon, 2018-12-03 at 14:48 +0100, jacopo mondi wrote:
> > Hi Lubomir,
> > 
> >   thanks for the patches
> > 
> > On Wed, Nov 28, 2018 at 06:19:13PM +0100, Lubomir Rintel wrote:
> > > Provide a dummy implementation when configured without
> > > CONFIG_VIDEO_V4L2_SUBDEV_API to avoid ifdef dance in the drivers
> > > that can
> > > be built either with or without the option.
> > > 
> > > Suggested-by: Jacopo Mondi <jacopo@...ndi.org>
> > > Signed-off-by: Lubomir Rintel <lkundrak@...sk>
> > > ---
> > >  include/media/v4l2-subdev.h | 11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > > 
> > > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-
> > > subdev.h
> > > index 9102d6ca566e..906e28011bb4 100644
> > > --- a/include/media/v4l2-subdev.h
> > > +++ b/include/media/v4l2-subdev.h
> > > @@ -967,6 +967,17 @@ static inline struct v4l2_rect
> > >  		pad = 0;
> > >  	return &cfg[pad].try_compose;
> > >  }
> > > +
> > > +#else /* !defined(CONFIG_VIDEO_V4L2_SUBDEV_API) */
> > > +
> > > +static inline struct v4l2_mbus_framefmt
> > > +*v4l2_subdev_get_try_format(struct v4l2_subdev *sd,
> > > +			    struct v4l2_subdev_pad_config *cfg,
> > > +			    unsigned int pad)
> > > +{
> > > +	return ERR_PTR(-ENOTTY);
> > > +}
> > > +
> > >  #endif
> > 
> > While at there, what about doing the same for get_try_crop and
> > get_try_compose? At lease provide stubs, I let you figure out if
> > you're willing to fix callers too, it seems there are quite a few of
> > them though
> > 
> > $ git grep v4l2_subdev_get_try* drivers/media/ | grep -v '_format' |
> > wc -l
> > 44
> 
> I'd be happy to do that. However, the drivers that use those seem to
> depend on CONFIG_VIDEO_V4L2_SUBDEV_API anyway. Should those
> dependencies be eventually done away with?
> 
> Please pardon my ignorance -- I don't actually understand why would
> anyone disable CONFIG_VIDEO_V4L2_SUBDEV_API.
> 
> I'll be following up with a v2 after I get a response from you. It will
> address locking issues found with smatch: one introduced by my patch
> and one that was there before.

Hans wrote a patch that enables VIDEO_V4L2_SUBDEV_API if
MEDIA_CAMERA_SUPPORT has been selected. So much of these #ifdefs will not
be needed going forward. It's an RFC for now, but that'd require a lot of
conditional compiling in drivers.

<URL:https://patchwork.linuxtv.org/patch/53370/>

The current use of the get_fmt callback serves the regular V4L2 API use, in
which case getting the try format is not meaningful.

-- 
Regards,

Sakari Ailus