[libcamera-devel] [PATCH] libcamera: Drop argument from LIBCAMERA_DECLARE_PRIVATE

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Sun, Apr 18, 2021 at 04:55:45PM +0200, Jacopo Mondi wrote:
> The LIBCAMERA_DECLARE_PRIVATE() macro, used by the library classes
> that inherits from libcamera::Extensible in order to implement the
> PIMPL pattern, expands to:
> 
> public:									\
> 	class Private;							\
> 	friend class Private;
> 
> The 'klass' argument is not used and it might confuse developers as
> it might hint that the class that defines the pattern's implementation
> can be freely named, while it is actually hardcoded to 'Private'.
> 
> Drop the argument from the macro definition.

I've intentionally kept the argument (I think this has actually been
discussed before) for symmetry with LIBCAMERA_DECLARE_PUBLIC(), and
perhaps, unconciously, wondering if it would be needed later. Those are
pretty weak reasons, and it could indeed be confusing for developers. I
don't mind much either way, but if you'd like to merge this patch,
you've forgotten to update the documentation of the macro in class.cpp.
Did doxygen remain silent ?

> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  include/libcamera/camera.h         | 2 +-
>  include/libcamera/camera_manager.h | 2 +-
>  include/libcamera/class.h          | 4 ++--
>  src/android/camera_buffer.h        | 2 +-
>  4 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> index 326b14d0ca01..d71641805c0a 100644
> --- a/include/libcamera/camera.h
> +++ b/include/libcamera/camera.h
> @@ -74,7 +74,7 @@ protected:
>  class Camera final : public Object, public std::enable_shared_from_this<Camera>,
>  		     public Extensible
>  {
> -	LIBCAMERA_DECLARE_PRIVATE(Camera)
> +	LIBCAMERA_DECLARE_PRIVATE()
>  
>  public:
>  	static std::shared_ptr<Camera> create(PipelineHandler *pipe,
> diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h
> index 35a59f0df4ca..c2f0b786da8e 100644
> --- a/include/libcamera/camera_manager.h
> +++ b/include/libcamera/camera_manager.h
> @@ -22,7 +22,7 @@ class Camera;
>  
>  class CameraManager : public Object, public Extensible
>  {
> -	LIBCAMERA_DECLARE_PRIVATE(CameraManager)
> +	LIBCAMERA_DECLARE_PRIVATE()
>  public:
>  	CameraManager();
>  	~CameraManager();
> diff --git a/include/libcamera/class.h b/include/libcamera/class.h
> index 920624d8e726..466114ecfaf4 100644
> --- a/include/libcamera/class.h
> +++ b/include/libcamera/class.h
> @@ -30,7 +30,7 @@ namespace libcamera {
>  #endif
>  
>  #ifndef __DOXYGEN__
> -#define LIBCAMERA_DECLARE_PRIVATE(klass)				\
> +#define LIBCAMERA_DECLARE_PRIVATE()					\
>  public:									\
>  	class Private;							\
>  	friend class Private;
> @@ -46,7 +46,7 @@ public:									\
>  	_o<Public>();
>  
>  #else
> -#define LIBCAMERA_DECLARE_PRIVATE(klass)
> +#define LIBCAMERA_DECLARE_PRIVATE()
>  #define LIBCAMERA_DECLARE_PUBLIC(klass)
>  #define LIBCAMERA_D_PTR(klass)
>  #define LIBCAMERA_O_PTR(klass)
> diff --git a/src/android/camera_buffer.h b/src/android/camera_buffer.h
> index 7e8970b49f49..c88124b2b3f3 100644
> --- a/src/android/camera_buffer.h
> +++ b/src/android/camera_buffer.h
> @@ -14,7 +14,7 @@
>  
>  class CameraBuffer final : public libcamera::Extensible
>  {
> -	LIBCAMERA_DECLARE_PRIVATE(CameraBuffer)
> +	LIBCAMERA_DECLARE_PRIVATE()
>  
>  public:
>  	CameraBuffer(buffer_handle_t camera3Buffer, int flags);

-- 
Regards,

Laurent Pinchart