[libcamera-devel] [PATCH v2] libcamera: controls: Suppress error message from ControlList::get()

[Naushir Patuck]

Hi Laurent,

On Wed, 20 Jul 2022 at 11:19, Laurent Pinchart <
laurent.pinchart at ideasonboard.com> wrote:

> Hi Naush,
>
> On Wed, Jul 20, 2022 at 09:17:45AM +0100, Naushir Patuck via
> libcamera-devel wrote:
> > On Wed, 20 Jul 2022 at 09:15, Naushir Patuck wrote:
> >
> > > Now that ControlList::get() returns a std::optional<T> to handle
> missing
> > > controls, the error log message in the call to ControlList::find() is
> > > unnecessary and likely invalid.
> > >
> > > Fix this by avoding the call to ControlList::find() from
> > > ControlList::get() and
> > > replacing with a call to the underlying std::unordered_map::find().
> > >
> > > Fixes: 1c4d48018505 ("libcamera: controls: Use std::optional to handle
> > > invalid control values")
> > > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > > ---
> > >  include/libcamera/controls.h | 7 ++++---
> > >  1 file changed, 4 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/include/libcamera/controls.h
> b/include/libcamera/controls.h
> > > index 192be78434dc..8362fce813fb 100644
> > > --- a/include/libcamera/controls.h
> > > +++ b/include/libcamera/controls.h
> > > @@ -376,11 +376,12 @@ public:
> > >         template<typename T>
> > >         std::optional<T> get(const Control<T> &ctrl) const
> > >         {
> > > -               const ControlValue *val = find(ctrl.id());
> > > -               if (!val)
> > > +               const auto entry = controls_.find(ctrl.id());
> > > +               if (entry == controls_.end())
> > >                         return std::nullopt;
> > >
> > > -               return val->get<T>();
> > > +               const ControlValue &val = entry->second;
> > > +               return val.get<T>();
> >
> > Bonus marks if somebody could explain to me why I need to cast
> > entry->second to a const for get() to work...?  If I don't, the following
> > statement:
> >
> > return entry->second.get<T>()
> >
> > returns the following compile error:
> >
> > ../include/libcamera/controls.h: In member function ‘std::optional<_Tp>
> > libcamera::ControlList::get(const libcamera::Control<T>&) const’:
> > ../include/libcamera/controls.h:384:33: error: expected
> primary-expression
> > before ‘>’ token
> >   384 |   return ((entry->second)).get<T>();
> >       |                                 ^
> > ../include/libcamera/controls.h:384:35: error: expected
> primary-expression
> > before ‘)’ token
> >   384 |   return ((entry->second)).get<T>();
> >
> > ?
>
> You need to write
>
>         return entry->second.template get<T>();
>
> Quoting the C++ standard,
>
> When the name of a member template specialization appears after . or ->
> in a postfix-expression, or after nested-name-specifier in a
> qualified-id, and the postfix-expression or qualified-id explicitly
> depends on a template-parameter (14.6.2), the member template name must
> be prefixed by the keyword template. Otherwise the name is assumed to
> name a non-template.
>
> The "The template disambiguator for dependent names" section in
> https://en.cppreference.com/w/cpp/language/dependent_name is easier to
> understand for us humans (I didn't say easy though).
>

Wow, I would have never ever guessed that as a solution :-)

Naush



>
> > >         }
> > >
> > >         template<typename T, typename V>
>
> --
> Regards,
>
> Laurent Pinchart
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20220721/5f45086b/attachment.htm>