[libcamera-devel,v2,3/5] libcamera: camera_manager: Apply clang thread safety annotation
diff mbox series

Message ID 20220620165027.549085-4-umang.jain@ideasonboard.com
State Superseded
Delegated to: Umang Jain
Headers show
Series
  • Apply clang thread safety annotation libcamera-core and v4l2
Related show

Commit Message

Umang Jain June 20, 2022, 4:50 p.m. UTC 
From: Hirokazu Honda <hiroh@chromium.org>

This annotates member functions and variables of
CameraManager::Private by clang thread safety annotations.

Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
---
 src/libcamera/camera_manager.cpp | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

Comments

Laurent Pinchart Aug. 28, 2022, 6:32 p.m. UTC | #1 
Hi Umang and Hiro,

Thank you for the patch.

On Mon, Jun 20, 2022 at 10:20:25PM +0530, Umang Jain via libcamera-devel wrote:
> From: Hirokazu Honda <hiroh@chromium.org>
> 
> This annotates member functions and variables of
> CameraManager::Private by clang thread safety annotations.
> 
> Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
>  src/libcamera/camera_manager.cpp | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
> index 70d73822..4f946516 100644
> --- a/src/libcamera/camera_manager.cpp
> +++ b/src/libcamera/camera_manager.cpp
> @@ -42,8 +42,8 @@ public:
>  
>  	int start();
>  	void addCamera(std::shared_ptr<Camera> camera,
> -		       const std::vector<dev_t> &devnums);
> -	void removeCamera(Camera *camera);
> +		       const std::vector<dev_t> &devnums) LIBCAMERA_TSA_EXCLUDES(mutex_);
> +	void removeCamera(Camera *camera) LIBCAMERA_TSA_EXCLUDES(mutex_);
>  
>  	/*
>  	 * This mutex protects
> @@ -52,8 +52,8 @@ public:
>  	 * - cameras_ and camerasByDevnum_ after initialization
>  	 */
>  	mutable Mutex mutex_;
> -	std::vector<std::shared_ptr<Camera>> cameras_;
> -	std::map<dev_t, std::weak_ptr<Camera>> camerasByDevnum_;
> +	std::vector<std::shared_ptr<Camera>> cameras_ LIBCAMERA_TSA_GUARDED_BY(mutex_);
> +	std::map<dev_t, std::weak_ptr<Camera>> camerasByDevnum_ LIBCAMERA_TSA_GUARDED_BY(mutex_);
>  
>  protected:
>  	void run() override;
> @@ -61,11 +61,11 @@ protected:
>  private:
>  	int init();
>  	void createPipelineHandlers();
> -	void cleanup();
> +	void cleanup() LIBCAMERA_TSA_EXCLUDES(mutex_);
>  
>  	ConditionVariable cv_;
> -	bool initialized_;
> -	int status_;
> +	bool initialized_ LIBCAMERA_TSA_GUARDED_BY(mutex_);
> +	int status_ LIBCAMERA_TSA_GUARDED_BY(mutex_);
>  
>  	std::unique_ptr<DeviceEnumerator> enumerator_;
>  
> @@ -87,7 +87,8 @@ int CameraManager::Private::start()
>  
>  	{
>  		MutexLocker locker(mutex_);
> -		cv_.wait(locker, [&] { return initialized_; });
> +		cv_.wait(locker,
> +			 [&]() LIBCAMERA_TSA_REQUIRES(mutex_) { return initialized_; });

		cv_.wait(locker, [&]() LIBCAMERA_TSA_REQUIRES(mutex_) {
				 return initialized_;
			 });

to match the usual style.

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

>  		status = status_;
>  	}
>

Patch
diff mbox series 

diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
index 70d73822..4f946516 100644
--- a/src/libcamera/camera_manager.cpp
+++ b/src/libcamera/camera_manager.cpp
@@ -42,8 +42,8 @@  public:
 
 	int start();
 	void addCamera(std::shared_ptr<Camera> camera,
-		       const std::vector<dev_t> &devnums);
-	void removeCamera(Camera *camera);
+		       const std::vector<dev_t> &devnums) LIBCAMERA_TSA_EXCLUDES(mutex_);
+	void removeCamera(Camera *camera) LIBCAMERA_TSA_EXCLUDES(mutex_);
 
 	/*
 	 * This mutex protects
@@ -52,8 +52,8 @@  public:
 	 * - cameras_ and camerasByDevnum_ after initialization
 	 */
 	mutable Mutex mutex_;
-	std::vector<std::shared_ptr<Camera>> cameras_;
-	std::map<dev_t, std::weak_ptr<Camera>> camerasByDevnum_;
+	std::vector<std::shared_ptr<Camera>> cameras_ LIBCAMERA_TSA_GUARDED_BY(mutex_);
+	std::map<dev_t, std::weak_ptr<Camera>> camerasByDevnum_ LIBCAMERA_TSA_GUARDED_BY(mutex_);
 
 protected:
 	void run() override;
@@ -61,11 +61,11 @@  protected:
 private:
 	int init();
 	void createPipelineHandlers();
-	void cleanup();
+	void cleanup() LIBCAMERA_TSA_EXCLUDES(mutex_);
 
 	ConditionVariable cv_;
-	bool initialized_;
-	int status_;
+	bool initialized_ LIBCAMERA_TSA_GUARDED_BY(mutex_);
+	int status_ LIBCAMERA_TSA_GUARDED_BY(mutex_);
 
 	std::unique_ptr<DeviceEnumerator> enumerator_;
 
@@ -87,7 +87,8 @@  int CameraManager::Private::start()
 
 	{
 		MutexLocker locker(mutex_);
-		cv_.wait(locker, [&] { return initialized_; });
+		cv_.wait(locker,
+			 [&]() LIBCAMERA_TSA_REQUIRES(mutex_) { return initialized_; });
 		status = status_;
 	}