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

Message ID 20190812124642.24287-6-laurent.pinchart@ideasonboard.com
State Superseded
Headers show
Series
  • Object & Thread enhancements
Related show

Commit Message

Laurent Pinchart Aug. 12, 2019, 12:46 p.m. UTC
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.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/bound_method.h | 11 +++++------
 include/libcamera/signal.h       | 12 +++++++-----
 src/libcamera/bound_method.cpp   |  6 ------
 3 files changed, 12 insertions(+), 17 deletions(-)

Comments

Niklas Söderlund Aug. 17, 2019, 2:24 p.m. UTC | #1
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@ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund@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@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch

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()) {