[libcamera-devel] [PATCH 1/3] libcamera: base: class: Expose Extensible private data to other classes

[Kieran Bingham]

Hi Laurent,

On 05/07/2021 00:28, Laurent Pinchart wrote:
> Despite sharing the same name, the private data class created by the
> Extensible design pattern and the C++ private access specifier have
> different goals. The latter specifies class members private to the
> class, while the former stores data not visible to the application.
> 
> There are use cases for accessing the private data class from other
> classes inside libcamera. Make this possible by exposing public _d()
> functions in the class deriving from Extensible. This won't allow access
> to the private data by applications as the definition of the Private
> class isn't visible outside of libcamera.

It almost makes me think the name for the D ptr type should be
'Internal', or 'Protected' rather than 'Private', as it's not private
(it can be accessed) - it's just simply not exposed ...

But I'm not opposed to keeping the names we have.
Ultimately it's the same concept [private: means private to the class,
Private means private to libcamera as a whole]


> The _d() functions need to be defined as template functions to delay
> their evaluation, as the static_cast() operator in the Extensible::_d()
> functions needs the Private class to be fully defined.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  include/libcamera/base/class.h | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/include/libcamera/base/class.h b/include/libcamera/base/class.h
> index a07dac057331..8212c3d4a5ae 100644
> --- a/include/libcamera/base/class.h
> +++ b/include/libcamera/base/class.h
> @@ -33,14 +33,24 @@ namespace libcamera {
>  #define LIBCAMERA_DECLARE_PRIVATE()					\
>  public:									\
>  	class Private;							\
> -	friend class Private;
> +	friend class Private;						\
> +	template <bool B = true>					\

Is this some template magic (hack?), to make it a template without
actually needing to be a template or have any template parameters?

If so it would be nice to state that in a comment so readers don't
wonder or query it.


> +	const Private *_d() const					\
> +	{								\
> +		return Extensible::_d<Private>();			\
> +	}								\
> +	template <bool B = true>					\
> +	Private *_d()							\
> +	{								\
> +		return Extensible::_d<Private>();			\
> +	}
>  
>  #define LIBCAMERA_DECLARE_PUBLIC(klass)					\
>  	friend class klass;						\
>  	using Public = klass;
>  
>  #define LIBCAMERA_D_PTR()						\
> -	_d<Private>();
> +	_d();
>  
>  #define LIBCAMERA_O_PTR()						\
>  	_o<Public>();
> 

Presumably there wouldn't ever be the need for the equivalent _o() as
that will only every be used from internally in the Private class.

--
Kieran