[libcamera-devel] [PATCH v2 2/5] cam: options: Add public method to invalided options
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Apr 30 18:41:21 CEST 2020
Hi Niklas,
On Tue, Apr 28, 2020 at 01:13:52AM +0200, Niklas Söderlund wrote:
> On 2020-04-28 01:53:40 +0300, Laurent Pinchart wrote:
> >
> > s/invalided/invalidate/ in the subject line.
> >
> > On Tue, Apr 28, 2020 at 12:05:26AM +0200, Niklas Söderlund wrote:
> > > Extend OptionsBase<T> with a public invalidate() method. This allows
> > > sub-classes to continue the interpreting of parsed options and
> > > invalidate them if they find problems with the result.
> > >
> > > Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> > > ---
> > > src/cam/options.cpp | 6 ++++++
> > > src/cam/options.h | 2 ++
> > > 2 files changed, 8 insertions(+)
> > >
> > > diff --git a/src/cam/options.cpp b/src/cam/options.cpp
> > > index 2c56eacf006e9857..77b3cc1f8c5a59e9 100644
> > > --- a/src/cam/options.cpp
> > > +++ b/src/cam/options.cpp
> > > @@ -64,6 +64,12 @@ const OptionValue &OptionsBase<T>::operator[](const T &opt) const
> > > return values_.find(opt)->second;
> > > }
> > >
> > > +template<typename T>
> > > +void OptionsBase<T>::invalidate()
> > > +{
> > > + valid_ = false;
> > > +}
> > > +
> > > template<typename T>
> > > bool OptionsBase<T>::parseValue(const T &opt, const Option &option,
> > > const char *optarg)
> > > diff --git a/src/cam/options.h b/src/cam/options.h
> > > index 5060fee0c26b896f..8ccbfee48f7ca0b5 100644
> > > --- a/src/cam/options.h
> > > +++ b/src/cam/options.h
> > > @@ -54,6 +54,8 @@ public:
> > > bool isSet(const T &opt) const;
> > > const OptionValue &operator[](const T &opt) const;
> > >
> > > + void invalidate();
> >
> > If this is only for subclasses, the method should be protected. I'm
> > tempted to go simpler and move the valid_ field protected. What do you
> > think ? If you want to keep the method I'd make it inline, as it will be
> > more efficient.
>
> My bad the commit message is wrong, it's not for subclassing. It's for
> processing the KeyValueParser::Options (which inherits from
> OptionsBase<T>) in subclasses of KeyValueParser.
>
> One could move valid_ to protected and the implement
> KeyValueParser::Options::invalidate() to hide it a bit more. But I don't
> see the harm in allowing even applications to examine the options parsed
> from the command line and if it does not like them marking the whole set
> as invalid. I would not however expose a setValid(bool valid) interface
> to applications.
>
> I have no strong feelings here so I'm open to any solution tho.
It's an internal API, doesn't matter too much. With the commit message
updated,
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > +
> > > private:
> > > friend class KeyValueParser;
> > > friend class OptionsParser;
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list