[libcamera-devel] libcamera: frame_buffer: Print errno on mmap() failure
diff mbox series

Message ID 20210809160445.179306-1-umang.jain@ideasonboard.com
State Accepted
Headers show
Series
  • [libcamera-devel] libcamera: frame_buffer: Print errno on mmap() failure
Related show

Commit Message

Umang Jain Aug. 9, 2021, 4:04 p.m. UTC
In mmap() error handling path, errno is stored but never printed
in the error log. Print it.

Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
---
 src/libcamera/framebuffer.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Laurent Pinchart Aug. 10, 2021, 1:25 a.m. UTC | #1
Hi Umang,

Thank you for the patch.

On Mon, Aug 09, 2021 at 09:34:45PM +0530, Umang Jain wrote:
> In mmap() error handling path, errno is stored but never printed
> in the error log. Print it.

It's not a bug as such, but it can indeed be useful for debugging.

> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>

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

> ---
>  src/libcamera/framebuffer.cpp | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp
> index 41e6e68f..280f64c6 100644
> --- a/src/libcamera/framebuffer.cpp
> +++ b/src/libcamera/framebuffer.cpp
> @@ -388,7 +388,8 @@ MappedFrameBuffer::MappedFrameBuffer(const FrameBuffer *buffer, int flags)
>  				     MAP_SHARED, plane.fd.fd(), 0);
>  		if (address == MAP_FAILED) {
>  			error_ = -errno;
> -			LOG(Buffer, Error) << "Failed to mmap plane";
> +			LOG(Buffer, Error) << "Failed to mmap plane: "
> +					   << strerror(-error_);
>  			break;
>  		}
>
Paul Elder Aug. 10, 2021, 4:55 a.m. UTC | #2
Hi Umang,

On Mon, Aug 09, 2021 at 09:34:45PM +0530, Umang Jain wrote:
> In mmap() error handling path, errno is stored but never printed
> in the error log. Print it.
> 
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> ---
>  src/libcamera/framebuffer.cpp | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp
> index 41e6e68f..280f64c6 100644
> --- a/src/libcamera/framebuffer.cpp
> +++ b/src/libcamera/framebuffer.cpp
> @@ -388,7 +388,8 @@ MappedFrameBuffer::MappedFrameBuffer(const FrameBuffer *buffer, int flags)
>  				     MAP_SHARED, plane.fd.fd(), 0);
>  		if (address == MAP_FAILED) {
>  			error_ = -errno;
> -			LOG(Buffer, Error) << "Failed to mmap plane";
> +			LOG(Buffer, Error) << "Failed to mmap plane: "
> +					   << strerror(-error_);
>  			break;
>  		}
>  
> -- 
> 2.31.1
>
Kieran Bingham Aug. 10, 2021, 8:29 a.m. UTC | #3
Hi Umang,

On 09/08/2021 17:04, Umang Jain wrote:
> In mmap() error handling path, errno is stored but never printed
> in the error log. Print it.

Good spot...

> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
>  src/libcamera/framebuffer.cpp | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp
> index 41e6e68f..280f64c6 100644
> --- a/src/libcamera/framebuffer.cpp
> +++ b/src/libcamera/framebuffer.cpp
> @@ -388,7 +388,8 @@ MappedFrameBuffer::MappedFrameBuffer(const FrameBuffer *buffer, int flags)
>  				     MAP_SHARED, plane.fd.fd(), 0);
>  		if (address == MAP_FAILED) {
>  			error_ = -errno;
> -			LOG(Buffer, Error) << "Failed to mmap plane";
> +			LOG(Buffer, Error) << "Failed to mmap plane: "
> +					   << strerror(-error_);

Should we say which plane?

It may not matter - as it's just a fairly bad failure if we hit this.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

>  			break;
>  		}
>  
>

Patch
diff mbox series

diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp
index 41e6e68f..280f64c6 100644
--- a/src/libcamera/framebuffer.cpp
+++ b/src/libcamera/framebuffer.cpp
@@ -388,7 +388,8 @@  MappedFrameBuffer::MappedFrameBuffer(const FrameBuffer *buffer, int flags)
 				     MAP_SHARED, plane.fd.fd(), 0);
 		if (address == MAP_FAILED) {
 			error_ = -errno;
-			LOG(Buffer, Error) << "Failed to mmap plane";
+			LOG(Buffer, Error) << "Failed to mmap plane: "
+					   << strerror(-error_);
 			break;
 		}