[libcamera-devel] hal: Fix comparison of integers of different signs
diff mbox series

Message ID 20190812110447.9411-1-laurent.pinchart@ideasonboard.com
State Accepted
Commit a6799dc5b9ded442152b3430e321d9b147b9d7fd
Headers show
Series
  • [libcamera-devel] hal: Fix comparison of integers of different signs
Related show

Commit Message

Laurent Pinchart Aug. 12, 2019, 11:04 a.m. UTC 
The CameraHalManager::getCameraInfo() validates the camera id it
receives from the camera service, and in doing so compares it with an
unsigned integer, generating a compiler error:

src/android/camera_hal_manager.cpp:121:9: error: comparison of integers of different signs: 'int' and 'unsigned int' [-Werror,-Wsign-compare]
        if (id >= numCameras() || id < 0) {
            ~~ ^  ~~~~~~~~~~~~

Fix this by turning the id into an unsigned int, as camera ids can't be
negative. If a negative id is received from the camera service it will
be converted to a large unsigned integer that will fail the comparison
with numCameras().

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/android/camera_hal_manager.cpp | 4 ++--
 src/android/camera_hal_manager.h   | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

Comments

Niklas Söderlund Aug. 12, 2019, 11:07 a.m. UTC | #1 
Hi Laurent,

Thanks for your patch.

On 2019-08-12 14:04:47 +0300, Laurent Pinchart wrote:
> The CameraHalManager::getCameraInfo() validates the camera id it
> receives from the camera service, and in doing so compares it with an
> unsigned integer, generating a compiler error:
> 
> src/android/camera_hal_manager.cpp:121:9: error: comparison of integers of different signs: 'int' and 'unsigned int' [-Werror,-Wsign-compare]
>         if (id >= numCameras() || id < 0) {
>             ~~ ^  ~~~~~~~~~~~~
> 
> Fix this by turning the id into an unsigned int, as camera ids can't be
> negative. If a negative id is received from the camera service it will
> be converted to a large unsigned integer that will fail the comparison
> with numCameras().
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

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

> ---
>  src/android/camera_hal_manager.cpp | 4 ++--
>  src/android/camera_hal_manager.h   | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp
> index 08c759dfd8a1..1e66f63240da 100644
> --- a/src/android/camera_hal_manager.cpp
> +++ b/src/android/camera_hal_manager.cpp
> @@ -113,12 +113,12 @@ unsigned int CameraHalManager::numCameras() const
>  	return cameraManager_->cameras().size();
>  }
>  
> -int CameraHalManager::getCameraInfo(int id, struct camera_info *info)
> +int CameraHalManager::getCameraInfo(unsigned int id, struct camera_info *info)
>  {
>  	if (!info)
>  		return -EINVAL;
>  
> -	if (id >= numCameras() || id < 0) {
> +	if (id >= numCameras()) {
>  		LOG(HAL, Error) << "Invalid camera id '" << id << "'";
>  		return -EINVAL;
>  	}
> diff --git a/src/android/camera_hal_manager.h b/src/android/camera_hal_manager.h
> index 8004aaf660f5..8228623aba90 100644
> --- a/src/android/camera_hal_manager.h
> +++ b/src/android/camera_hal_manager.h
> @@ -30,7 +30,7 @@ public:
>  	int close(CameraProxy *proxy);
>  
>  	unsigned int numCameras() const;
> -	int getCameraInfo(int id, struct camera_info *info);
> +	int getCameraInfo(unsigned int id, struct camera_info *info);
>  
>  private:
>  	void run() override;
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Jacopo Mondi Aug. 12, 2019, 12:02 p.m. UTC | #2 
Hi Laurent,

On Mon, Aug 12, 2019 at 02:04:47PM +0300, Laurent Pinchart wrote:
> The CameraHalManager::getCameraInfo() validates the camera id it
> receives from the camera service, and in doing so compares it with an
> unsigned integer, generating a compiler error:
>
> src/android/camera_hal_manager.cpp:121:9: error: comparison of integers of different signs: 'int' and 'unsigned int' [-Werror,-Wsign-compare]
>         if (id >= numCameras() || id < 0) {
>             ~~ ^  ~~~~~~~~~~~~
>
> Fix this by turning the id into an unsigned int, as camera ids can't be
> negative. If a negative id is received from the camera service it will
> be converted to a large unsigned integer that will fail the comparison
> with numCameras().

My bad, I missed it!
Thanks for fixing

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
  j

>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/android/camera_hal_manager.cpp | 4 ++--
>  src/android/camera_hal_manager.h   | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp
> index 08c759dfd8a1..1e66f63240da 100644
> --- a/src/android/camera_hal_manager.cpp
> +++ b/src/android/camera_hal_manager.cpp
> @@ -113,12 +113,12 @@ unsigned int CameraHalManager::numCameras() const
>  	return cameraManager_->cameras().size();
>  }
>
> -int CameraHalManager::getCameraInfo(int id, struct camera_info *info)
> +int CameraHalManager::getCameraInfo(unsigned int id, struct camera_info *info)
>  {
>  	if (!info)
>  		return -EINVAL;
>
> -	if (id >= numCameras() || id < 0) {
> +	if (id >= numCameras()) {
>  		LOG(HAL, Error) << "Invalid camera id '" << id << "'";
>  		return -EINVAL;
>  	}
> diff --git a/src/android/camera_hal_manager.h b/src/android/camera_hal_manager.h
> index 8004aaf660f5..8228623aba90 100644
> --- a/src/android/camera_hal_manager.h
> +++ b/src/android/camera_hal_manager.h
> @@ -30,7 +30,7 @@ public:
>  	int close(CameraProxy *proxy);
>
>  	unsigned int numCameras() const;
> -	int getCameraInfo(int id, struct camera_info *info);
> +	int getCameraInfo(unsigned int id, struct camera_info *info);
>
>  private:
>  	void run() override;
> --
> Regards,
>
> Laurent Pinchart
>

Patch
diff mbox series 

diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp
index 08c759dfd8a1..1e66f63240da 100644
--- a/src/android/camera_hal_manager.cpp
+++ b/src/android/camera_hal_manager.cpp
@@ -113,12 +113,12 @@  unsigned int CameraHalManager::numCameras() const
 	return cameraManager_->cameras().size();
 }
 
-int CameraHalManager::getCameraInfo(int id, struct camera_info *info)
+int CameraHalManager::getCameraInfo(unsigned int id, struct camera_info *info)
 {
 	if (!info)
 		return -EINVAL;
 
-	if (id >= numCameras() || id < 0) {
+	if (id >= numCameras()) {
 		LOG(HAL, Error) << "Invalid camera id '" << id << "'";
 		return -EINVAL;
 	}
diff --git a/src/android/camera_hal_manager.h b/src/android/camera_hal_manager.h
index 8004aaf660f5..8228623aba90 100644
--- a/src/android/camera_hal_manager.h
+++ b/src/android/camera_hal_manager.h
@@ -30,7 +30,7 @@  public:
 	int close(CameraProxy *proxy);
 
 	unsigned int numCameras() const;
-	int getCameraInfo(int id, struct camera_info *info);
+	int getCameraInfo(unsigned int id, struct camera_info *info);
 
 private:
 	void run() override;