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

[Laurent Pinchart]

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).

> >         }
> >
> >         template<typename T, typename V>

-- 
Regards,

Laurent Pinchart