[libcamera-devel] [PATCH 05/18] libcamera: bound_method: Decouple from Signal implementation

[Niklas Söderlund]

Hi Laurent,

Thanks for your work.

On 2019-08-12 15:46:29 +0300, Laurent Pinchart wrote:
> To make the BoundMethod classes more generic, replace direct access to
> private member from Signal classes with accessors or helper functions.
> This allows removal of friend statements from the BoundMethod classes.

I love to lose these toxic friends ;-)

> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>

> ---
>  include/libcamera/bound_method.h | 11 +++++------
>  include/libcamera/signal.h       | 12 +++++++-----
>  src/libcamera/bound_method.cpp   |  6 ------
>  3 files changed, 12 insertions(+), 17 deletions(-)
> 
> diff --git a/include/libcamera/bound_method.h b/include/libcamera/bound_method.h
> index 38c44b923ba1..54c40fc52e3b 100644
> --- a/include/libcamera/bound_method.h
> +++ b/include/libcamera/bound_method.h
> @@ -13,9 +13,6 @@
>  namespace libcamera {
>  
>  class Object;
> -template<typename... Args>
> -class Signal;
> -class SignalBase;
>  
>  class BoundMethodBase
>  {
> @@ -28,7 +25,7 @@ public:
>  	bool match(T *obj) { return obj == obj_; }
>  	bool match(Object *object) { return object == object_; }
>  
> -	void disconnect(SignalBase *signal);
> +	Object *object() const { return object_; }
>  
>  	void activatePack(void *pack);
>  	virtual void invokePack(void *pack) = 0;
> @@ -93,6 +90,8 @@ public:
>  	BoundMemberMethod(T *obj, Object *object, void (T::*func)(Args...))
>  		: BoundMethodArgs<Args...>(obj, object), func_(func) {}
>  
> +	bool match(void (T::*func)(Args...)) const { return func == func_; }
> +
>  	void activate(Args... args)
>  	{
>  		if (this->object_)
> @@ -107,7 +106,6 @@ public:
>  	}
>  
>  private:
> -	friend class Signal<Args...>;
>  	void (T::*func_)(Args...);
>  };
>  
> @@ -118,11 +116,12 @@ public:
>  	BoundStaticMethod(void (*func)(Args...))
>  		: BoundMethodArgs<Args...>(nullptr, nullptr), func_(func) {}
>  
> +	bool match(void (*func)(Args...)) const { return func == func_; }
> +
>  	void activate(Args... args) { (*func_)(args...); }
>  	void invoke(Args... args) {}
>  
>  private:
> -	friend class Signal<Args...>;
>  	void (*func_)(Args...);
>  };
>  
> diff --git a/include/libcamera/signal.h b/include/libcamera/signal.h
> index 3b6de30f7d35..b8a60281cceb 100644
> --- a/include/libcamera/signal.h
> +++ b/include/libcamera/signal.h
> @@ -46,7 +46,9 @@ public:
>  	~Signal()
>  	{
>  		for (BoundMethodBase *slot : slots_) {
> -			slot->disconnect(this);
> +			Object *object = slot->object();
> +			if (object)
> +				object->disconnect(this);
>  			delete slot;
>  		}
>  	}
> @@ -95,11 +97,11 @@ public:
>  			/*
>  			 * If the object matches the slot, the slot is
>  			 * guaranteed to be a member slot, so we can safely
> -			 * cast it to BoundMemberMethod<T, Args...> and access its
> -			 * func_ member.
> +			 * cast it to BoundMemberMethod<T, Args...> to match
> +			 * func.
>  			 */
>  			if (slot->match(obj) &&
> -			    static_cast<BoundMemberMethod<T, Args...> *>(slot)->func_ == func) {
> +			    static_cast<BoundMemberMethod<T, Args...> *>(slot)->match(func)) {
>  				iter = slots_.erase(iter);
>  				delete slot;
>  			} else {
> @@ -113,7 +115,7 @@ public:
>  		for (auto iter = slots_.begin(); iter != slots_.end(); ) {
>  			BoundMethodArgs<Args...> *slot = *iter;
>  			if (slot->match(nullptr) &&
> -			    static_cast<BoundStaticMethod<Args...> *>(slot)->func_ == func) {
> +			    static_cast<BoundStaticMethod<Args...> *>(slot)->match(func)) {
>  				iter = slots_.erase(iter);
>  				delete slot;
>  			} else {
> diff --git a/src/libcamera/bound_method.cpp b/src/libcamera/bound_method.cpp
> index 0a2d61a6c805..23b8f4122283 100644
> --- a/src/libcamera/bound_method.cpp
> +++ b/src/libcamera/bound_method.cpp
> @@ -13,12 +13,6 @@
>  
>  namespace libcamera {
>  
> -void BoundMethodBase::disconnect(SignalBase *signal)
> -{
> -	if (object_)
> -		object_->disconnect(signal);
> -}
> -
>  void BoundMethodBase::activatePack(void *pack)
>  {
>  	if (Thread::current() == object_->thread()) {
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund