[libcamera-devel] [PATCH v1 5/6] libcamera: Don't use emitter object pointer argument to slot

[Umang Jain]

Hi Laurent,

On 8/27/21 8:08 AM, Laurent Pinchart wrote:
> In many cases, the emitter object passed as a pointer from signals to
> slots is also available as a class member. Use the class member when
> this occurs, to prepare for removal of the emitter object pointer from
> signals.
>
> In test/event.cpp, this additionally requires moving the EventNotifier
> to a class member.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>


Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>

> ---
>   src/libcamera/ipc_pipe_unixsocket.cpp           |  4 ++--
>   test/event-thread.cpp                           |  4 ++--
>   test/event.cpp                                  | 17 +++++++++++------
>   test/ipa/ipa_interface_test.cpp                 |  4 ++--
>   test/ipc/unixsocket.cpp                         |  8 ++++----
>   test/ipc/unixsocket_ipc.cpp                     |  4 ++--
>   .../module_ipa_proxy_worker.cpp.tmpl            |  4 ++--
>   7 files changed, 25 insertions(+), 20 deletions(-)
>
> diff --git a/src/libcamera/ipc_pipe_unixsocket.cpp b/src/libcamera/ipc_pipe_unixsocket.cpp
> index 4511775fb467..38bcc30a21ed 100644
> --- a/src/libcamera/ipc_pipe_unixsocket.cpp
> +++ b/src/libcamera/ipc_pipe_unixsocket.cpp
> @@ -82,10 +82,10 @@ int IPCPipeUnixSocket::sendAsync(const IPCMessage &data)
>   	return 0;
>   }
>   
> -void IPCPipeUnixSocket::readyRead(IPCUnixSocket *socket)
> +void IPCPipeUnixSocket::readyRead([[maybe_unused]] IPCUnixSocket *socket)
>   {
>   	IPCUnixSocket::Payload payload;
> -	int ret = socket->receive(&payload);
> +	int ret = socket_->receive(&payload);
>   	if (ret) {
>   		LOG(IPCPipe, Error) << "Receive message failed" << ret;
>   		return;
> diff --git a/test/event-thread.cpp b/test/event-thread.cpp
> index 575261664c2f..12021710ef41 100644
> --- a/test/event-thread.cpp
> +++ b/test/event-thread.cpp
> @@ -66,9 +66,9 @@ public:
>   	}
>   
>   private:
> -	void readReady(EventNotifier *notifier)
> +	void readReady([[maybe_unused]] EventNotifier *notifier)
>   	{
> -		size_ = read(notifier->fd(), data_, sizeof(data_));
> +		size_ = read(notifier_->fd(), data_, sizeof(data_));
>   		notified_ = true;
>   	}
>   
> diff --git a/test/event.cpp b/test/event.cpp
> index c2274344b7f0..e338335c11e8 100644
> --- a/test/event.cpp
> +++ b/test/event.cpp
> @@ -22,14 +22,16 @@ using namespace libcamera;
>   class EventTest : public Test
>   {
>   protected:
> -	void readReady(EventNotifier *notifier)
> +	void readReady([[maybe_unused]] EventNotifier *notifier)
>   	{
> -		size_ = read(notifier->fd(), data_, sizeof(data_));
> +		size_ = read(notifier_->fd(), data_, sizeof(data_));
>   		notified_ = true;
>   	}
>   
>   	int init()
>   	{
> +		notifier_ = nullptr;
> +
>   		return pipe(pipefd_);
>   	}
>   
> @@ -40,8 +42,8 @@ protected:
>   		Timer timeout;
>   		ssize_t ret;
>   
> -		EventNotifier readNotifier(pipefd_[0], EventNotifier::Read);
> -		readNotifier.activated.connect(this, &EventTest::readReady);
> +		notifier_ = new EventNotifier(pipefd_[0], EventNotifier::Read);
> +		notifier_->activated.connect(this, &EventTest::readReady);
>   
>   		/* Test read notification with data. */
>   		memset(data_, 0, sizeof(data_));
> @@ -76,7 +78,7 @@ protected:
>   
>   		/* Test read notifier disabling. */
>   		notified_ = false;
> -		readNotifier.setEnabled(false);
> +		notifier_->setEnabled(false);
>   
>   		ret = write(pipefd_[1], data.data(), data.size());
>   		if (ret < 0) {
> @@ -95,7 +97,7 @@ protected:
>   
>   		/* Test read notifier enabling. */
>   		notified_ = false;
> -		readNotifier.setEnabled(true);
> +		notifier_->setEnabled(true);
>   
>   		timeout.start(100);
>   		dispatcher->processEvents();
> @@ -111,6 +113,8 @@ protected:
>   
>   	void cleanup()
>   	{
> +		delete notifier_;
> +
>   		close(pipefd_[0]);
>   		close(pipefd_[1]);
>   	}
> @@ -118,6 +122,7 @@ protected:
>   private:
>   	int pipefd_[2];
>   
> +	EventNotifier *notifier_;
>   	bool notified_;
>   	char data_[16];
>   	ssize_t size_;
> diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp
> index ee9f26510784..0ee51f71fd87 100644
> --- a/test/ipa/ipa_interface_test.cpp
> +++ b/test/ipa/ipa_interface_test.cpp
> @@ -153,9 +153,9 @@ protected:
>   	}
>   
>   private:
> -	void readTrace(EventNotifier *notifier)
> +	void readTrace([[maybe_unused]] EventNotifier *notifier)
>   	{
> -		ssize_t s = read(notifier->fd(), &trace_, sizeof(trace_));
> +		ssize_t s = read(notifier_->fd(), &trace_, sizeof(trace_));
>   		if (s < 0) {
>   			int ret = errno;
>   			cerr << "Failed to read from IPA test FIFO at '"
> diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp
> index aa35c8f071f1..6507fb12d74b 100644
> --- a/test/ipc/unixsocket.cpp
> +++ b/test/ipc/unixsocket.cpp
> @@ -68,12 +68,12 @@ public:
>   	}
>   
>   private:
> -	void readyRead(IPCUnixSocket *ipc)
> +	void readyRead([[maybe_unused]] IPCUnixSocket *ipc)
>   	{
>   		IPCUnixSocket::Payload message, response;
>   		int ret;
>   
> -		ret = ipc->receive(&message);
> +		ret = ipc_.receive(&message);
>   		if (ret) {
>   			cerr << "Receive message failed: " << ret << endl;
>   			return;
> @@ -447,14 +447,14 @@ private:
>   		return 0;
>   	}
>   
> -	void readyRead(IPCUnixSocket *ipc)
> +	void readyRead([[maybe_unused]] IPCUnixSocket *ipc)
>   	{
>   		if (!callResponse_) {
>   			cerr << "Read ready without expecting data, fail." << endl;
>   			return;
>   		}
>   
> -		if (ipc->receive(callResponse_)) {
> +		if (ipc_.receive(callResponse_)) {
>   			cerr << "Receive message failed" << endl;
>   			return;
>   		}
> diff --git a/test/ipc/unixsocket_ipc.cpp b/test/ipc/unixsocket_ipc.cpp
> index 6fe7fd9b8fc5..60317a4956b8 100644
> --- a/test/ipc/unixsocket_ipc.cpp
> +++ b/test/ipc/unixsocket_ipc.cpp
> @@ -65,12 +65,12 @@ public:
>   	}
>   
>   private:
> -	void readyRead(IPCUnixSocket *ipc)
> +	void readyRead([[maybe_unused]] IPCUnixSocket *ipc)
>   	{
>   		IPCUnixSocket::Payload message;
>   		int ret;
>   
> -		ret = ipc->receive(&message);
> +		ret = ipc_.receive(&message);
>   		if (ret) {
>   			cerr << "Receive message failed: " << ret << endl;
>   			return;
> diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl
> index 8a88bd467da7..b4cd1aa9e823 100644
> --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl
> +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl
> @@ -57,10 +57,10 @@ public:
>   
>   	~{{proxy_worker_name}}() {}
>   
> -	void readyRead(IPCUnixSocket *socket)
> +	void readyRead([[maybe_unused]] IPCUnixSocket *socket)
>   	{
>   		IPCUnixSocket::Payload _message;
> -		int _retRecv = socket->receive(&_message);
> +		int _retRecv = socket_.receive(&_message);
>   		if (_retRecv) {
>   			LOG({{proxy_worker_name}}, Error)
>   				<< "Receive message failed: " << _retRecv;