[libcamera-devel,v4,1/3] cam: event_loop: Rename event_ to base_
diff mbox series

Message ID 20210202221051.1740237-2-niklas.soderlund@ragnatech.se
State Accepted
Headers show
Series
  • cam: Fix races in event loop and long request processing times
Related show

Commit Message

Niklas Söderlund Feb. 2, 2021, 10:10 p.m. UTC 
The 'event' variable name is usually used for events added to the base
event loop, not the loop itself. Rename the struct event_base member to
base_ as a preparation for future work adding events to the loop.

There is no functional change.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 src/cam/event_loop.cpp | 8 ++++----
 src/cam/event_loop.h   | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

Comments

Jacopo Mondi Feb. 3, 2021, 11:31 a.m. UTC | #1 
Hi Niklas,

On Tue, Feb 02, 2021 at 11:10:49PM +0100, Niklas Söderlund wrote:
> The 'event' variable name is usually used for events added to the base
> event loop, not the loop itself. Rename the struct event_base member to
> base_ as a preparation for future work adding events to the loop.
>
> There is no functional change.
>

I would have used eventBase_ as 'event' in name is still relevant.

It's ok, just tastes
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
  j

> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
>  src/cam/event_loop.cpp | 8 ++++----
>  src/cam/event_loop.h   | 2 +-
>  2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/src/cam/event_loop.cpp b/src/cam/event_loop.cpp
> index 94c5d1d362455f33..3a2b665abdc063de 100644
> --- a/src/cam/event_loop.cpp
> +++ b/src/cam/event_loop.cpp
> @@ -18,7 +18,7 @@ EventLoop::EventLoop()
>  	assert(!instance_);
>
>  	evthread_use_pthreads();
> -	event_ = event_base_new();
> +	base_ = event_base_new();
>  	instance_ = this;
>  }
>
> @@ -26,7 +26,7 @@ EventLoop::~EventLoop()
>  {
>  	instance_ = nullptr;
>
> -	event_base_free(event_);
> +	event_base_free(base_);
>  	libevent_global_shutdown();
>  }
>
> @@ -42,7 +42,7 @@ int EventLoop::exec()
>
>  	while (!exit_.load(std::memory_order_acquire)) {
>  		dispatchCalls();
> -		event_base_loop(event_, EVLOOP_NO_EXIT_ON_EMPTY);
> +		event_base_loop(base_, EVLOOP_NO_EXIT_ON_EMPTY);
>  	}
>
>  	return exitCode_;
> @@ -57,7 +57,7 @@ void EventLoop::exit(int code)
>
>  void EventLoop::interrupt()
>  {
> -	event_base_loopbreak(event_);
> +	event_base_loopbreak(base_);
>  }
>
>  void EventLoop::callLater(const std::function<void()> &func)
> diff --git a/src/cam/event_loop.h b/src/cam/event_loop.h
> index 408073c50594d09d..d0d5b5a53c830670 100644
> --- a/src/cam/event_loop.h
> +++ b/src/cam/event_loop.h
> @@ -30,7 +30,7 @@ public:
>  private:
>  	static EventLoop *instance_;
>
> -	struct event_base *event_;
> +	struct event_base *base_;
>  	std::atomic<bool> exit_;
>  	int exitCode_;
>
> --
> 2.30.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Niklas Söderlund Feb. 3, 2021, 12:31 p.m. UTC | #2 
Hi Jacopo,

On 2021-02-03 12:31:08 +0100, Jacopo Mondi wrote:
> Hi Niklas,
> 
> On Tue, Feb 02, 2021 at 11:10:49PM +0100, Niklas Söderlund wrote:
> > The 'event' variable name is usually used for events added to the base
> > event loop, not the loop itself. Rename the struct event_base member to
> > base_ as a preparation for future work adding events to the loop.
> >
> > There is no functional change.
> >
> 
> I would have used eventBase_ as 'event' in name is still relevant.

I used base as that what's used in the libevent documentation, but I 
agree it's just taste.

> 
> It's ok, just tastes
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> 
> Thanks
>   j
> 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > ---
> >  src/cam/event_loop.cpp | 8 ++++----
> >  src/cam/event_loop.h   | 2 +-
> >  2 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/src/cam/event_loop.cpp b/src/cam/event_loop.cpp
> > index 94c5d1d362455f33..3a2b665abdc063de 100644
> > --- a/src/cam/event_loop.cpp
> > +++ b/src/cam/event_loop.cpp
> > @@ -18,7 +18,7 @@ EventLoop::EventLoop()
> >  	assert(!instance_);
> >
> >  	evthread_use_pthreads();
> > -	event_ = event_base_new();
> > +	base_ = event_base_new();
> >  	instance_ = this;
> >  }
> >
> > @@ -26,7 +26,7 @@ EventLoop::~EventLoop()
> >  {
> >  	instance_ = nullptr;
> >
> > -	event_base_free(event_);
> > +	event_base_free(base_);
> >  	libevent_global_shutdown();
> >  }
> >
> > @@ -42,7 +42,7 @@ int EventLoop::exec()
> >
> >  	while (!exit_.load(std::memory_order_acquire)) {
> >  		dispatchCalls();
> > -		event_base_loop(event_, EVLOOP_NO_EXIT_ON_EMPTY);
> > +		event_base_loop(base_, EVLOOP_NO_EXIT_ON_EMPTY);
> >  	}
> >
> >  	return exitCode_;
> > @@ -57,7 +57,7 @@ void EventLoop::exit(int code)
> >
> >  void EventLoop::interrupt()
> >  {
> > -	event_base_loopbreak(event_);
> > +	event_base_loopbreak(base_);
> >  }
> >
> >  void EventLoop::callLater(const std::function<void()> &func)
> > diff --git a/src/cam/event_loop.h b/src/cam/event_loop.h
> > index 408073c50594d09d..d0d5b5a53c830670 100644
> > --- a/src/cam/event_loop.h
> > +++ b/src/cam/event_loop.h
> > @@ -30,7 +30,7 @@ public:
> >  private:
> >  	static EventLoop *instance_;
> >
> > -	struct event_base *event_;
> > +	struct event_base *base_;
> >  	std::atomic<bool> exit_;
> >  	int exitCode_;
> >
> > --
> > 2.30.0
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel@lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
Laurent Pinchart Feb. 3, 2021, 11:47 p.m. UTC | #3 
Hi Niklas,

Thank you for the patch.

On Tue, Feb 02, 2021 at 11:10:49PM +0100, Niklas Söderlund wrote:
> The 'event' variable name is usually used for events added to the base
> event loop, not the loop itself. Rename the struct event_base member to
> base_ as a preparation for future work adding events to the loop.
> 
> There is no functional change.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

As Jacopo mentioned, this is a matter of taste, so

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

I however wonder if this patch is still needed or if it was only
required for previous versions of the series, as you don't introduce any
event_ variable in 2/3 or 3/3.

> ---
>  src/cam/event_loop.cpp | 8 ++++----
>  src/cam/event_loop.h   | 2 +-
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/src/cam/event_loop.cpp b/src/cam/event_loop.cpp
> index 94c5d1d362455f33..3a2b665abdc063de 100644
> --- a/src/cam/event_loop.cpp
> +++ b/src/cam/event_loop.cpp
> @@ -18,7 +18,7 @@ EventLoop::EventLoop()
>  	assert(!instance_);
>  
>  	evthread_use_pthreads();
> -	event_ = event_base_new();
> +	base_ = event_base_new();
>  	instance_ = this;
>  }
>  
> @@ -26,7 +26,7 @@ EventLoop::~EventLoop()
>  {
>  	instance_ = nullptr;
>  
> -	event_base_free(event_);
> +	event_base_free(base_);
>  	libevent_global_shutdown();
>  }
>  
> @@ -42,7 +42,7 @@ int EventLoop::exec()
>  
>  	while (!exit_.load(std::memory_order_acquire)) {
>  		dispatchCalls();
> -		event_base_loop(event_, EVLOOP_NO_EXIT_ON_EMPTY);
> +		event_base_loop(base_, EVLOOP_NO_EXIT_ON_EMPTY);
>  	}
>  
>  	return exitCode_;
> @@ -57,7 +57,7 @@ void EventLoop::exit(int code)
>  
>  void EventLoop::interrupt()
>  {
> -	event_base_loopbreak(event_);
> +	event_base_loopbreak(base_);
>  }
>  
>  void EventLoop::callLater(const std::function<void()> &func)
> diff --git a/src/cam/event_loop.h b/src/cam/event_loop.h
> index 408073c50594d09d..d0d5b5a53c830670 100644
> --- a/src/cam/event_loop.h
> +++ b/src/cam/event_loop.h
> @@ -30,7 +30,7 @@ public:
>  private:
>  	static EventLoop *instance_;
>  
> -	struct event_base *event_;
> +	struct event_base *base_;
>  	std::atomic<bool> exit_;
>  	int exitCode_;
>

Patch
diff mbox series 

diff --git a/src/cam/event_loop.cpp b/src/cam/event_loop.cpp
index 94c5d1d362455f33..3a2b665abdc063de 100644
--- a/src/cam/event_loop.cpp
+++ b/src/cam/event_loop.cpp
@@ -18,7 +18,7 @@  EventLoop::EventLoop()
 	assert(!instance_);
 
 	evthread_use_pthreads();
-	event_ = event_base_new();
+	base_ = event_base_new();
 	instance_ = this;
 }
 
@@ -26,7 +26,7 @@  EventLoop::~EventLoop()
 {
 	instance_ = nullptr;
 
-	event_base_free(event_);
+	event_base_free(base_);
 	libevent_global_shutdown();
 }
 
@@ -42,7 +42,7 @@  int EventLoop::exec()
 
 	while (!exit_.load(std::memory_order_acquire)) {
 		dispatchCalls();
-		event_base_loop(event_, EVLOOP_NO_EXIT_ON_EMPTY);
+		event_base_loop(base_, EVLOOP_NO_EXIT_ON_EMPTY);
 	}
 
 	return exitCode_;
@@ -57,7 +57,7 @@  void EventLoop::exit(int code)
 
 void EventLoop::interrupt()
 {
-	event_base_loopbreak(event_);
+	event_base_loopbreak(base_);
 }
 
 void EventLoop::callLater(const std::function<void()> &func)
diff --git a/src/cam/event_loop.h b/src/cam/event_loop.h
index 408073c50594d09d..d0d5b5a53c830670 100644
--- a/src/cam/event_loop.h
+++ b/src/cam/event_loop.h
@@ -30,7 +30,7 @@  public:
 private:
 	static EventLoop *instance_;
 
-	struct event_base *event_;
+	struct event_base *base_;
 	std::atomic<bool> exit_;
 	int exitCode_;