[2/3] libcamera: media_device: Use MediaLink string helper
diff mbox series

Message ID 20240815210925.4172287-3-kieran.bingham@ideasonboard.com
State Superseded
Headers show
Series
  • MediaLink: Provide a string representation
Related show

Commit Message

Kieran Bingham Aug. 15, 2024, 9:09 p.m. UTC
Replace the two open-coded implementations of a link representation
with the operator<< overload string representation to simplify
the code and unify appearance of reporting MediaLinks.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 src/libcamera/media_device.cpp | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

Comments

Laurent Pinchart Aug. 15, 2024, 9:18 p.m. UTC | #1
On Thu, Aug 15, 2024 at 10:09:24PM +0100, Kieran Bingham wrote:
> Replace the two open-coded implementations of a link representation
> with the operator<< overload string representation to simplify
> the code and unify appearance of reporting MediaLinks.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  src/libcamera/media_device.cpp | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp
> index bd054552a093..c1ea58996fa8 100644
> --- a/src/libcamera/media_device.cpp
> +++ b/src/libcamera/media_device.cpp
> @@ -817,21 +817,15 @@ int MediaDevice::setupLink(const MediaLink *link, unsigned int flags)
>  	int ret = ioctl(fd_.get(), MEDIA_IOC_SETUP_LINK, &linkDesc);
>  	if (ret) {
>  		ret = -errno;
> +
>  		LOG(MediaDevice, Error)
> -			<< "Failed to setup link "
> -			<< source->entity()->name() << "["
> -			<< source->index() << "] -> "
> -			<< sink->entity()->name() << "["
> -			<< sink->index() << "]: "
> +			<< "Failed to setup link " << *link << ": "
>  			<< strerror(-ret);
> +

I'm not sure I would have added the blank lines, but that's fine too.

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

>  		return ret;
>  	}
>  
> -	LOG(MediaDevice, Debug)
> -		<< source->entity()->name() << "["
> -		<< source->index() << "] -> "
> -		<< sink->entity()->name() << "["
> -		<< sink->index() << "]: " << flags;
> +	LOG(MediaDevice, Debug) << *link << ": " << flags;
>  
>  	return 0;
>  }
Stefan Klug Sept. 12, 2024, 8:55 a.m. UTC | #2
Hi Kieran,

Thank you for the patch. 

On Thu, Aug 15, 2024 at 10:09:24PM +0100, Kieran Bingham wrote:
> Replace the two open-coded implementations of a link representation
> with the operator<< overload string representation to simplify
> the code and unify appearance of reporting MediaLinks.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  src/libcamera/media_device.cpp | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp
> index bd054552a093..c1ea58996fa8 100644
> --- a/src/libcamera/media_device.cpp
> +++ b/src/libcamera/media_device.cpp
> @@ -817,21 +817,15 @@ int MediaDevice::setupLink(const MediaLink *link, unsigned int flags)
>  	int ret = ioctl(fd_.get(), MEDIA_IOC_SETUP_LINK, &linkDesc);
>  	if (ret) {
>  		ret = -errno;
> +
>  		LOG(MediaDevice, Error)
> -			<< "Failed to setup link "
> -			<< source->entity()->name() << "["
> -			<< source->index() << "] -> "
> -			<< sink->entity()->name() << "["
> -			<< sink->index() << "]: "
> +			<< "Failed to setup link " << *link << ": "
>  			<< strerror(-ret);
> +

Ohh, so much easier to read.

Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com> 

Regards,
Stefan

>  		return ret;
>  	}
>  
> -	LOG(MediaDevice, Debug)
> -		<< source->entity()->name() << "["
> -		<< source->index() << "] -> "
> -		<< sink->entity()->name() << "["
> -		<< sink->index() << "]: " << flags;
> +	LOG(MediaDevice, Debug) << *link << ": " << flags;
>  
>  	return 0;
>  }
> -- 
> 2.34.1
>

Patch
diff mbox series

diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp
index bd054552a093..c1ea58996fa8 100644
--- a/src/libcamera/media_device.cpp
+++ b/src/libcamera/media_device.cpp
@@ -817,21 +817,15 @@  int MediaDevice::setupLink(const MediaLink *link, unsigned int flags)
 	int ret = ioctl(fd_.get(), MEDIA_IOC_SETUP_LINK, &linkDesc);
 	if (ret) {
 		ret = -errno;
+
 		LOG(MediaDevice, Error)
-			<< "Failed to setup link "
-			<< source->entity()->name() << "["
-			<< source->index() << "] -> "
-			<< sink->entity()->name() << "["
-			<< sink->index() << "]: "
+			<< "Failed to setup link " << *link << ": "
 			<< strerror(-ret);
+
 		return ret;
 	}
 
-	LOG(MediaDevice, Debug)
-		<< source->entity()->name() << "["
-		<< source->index() << "] -> "
-		<< sink->entity()->name() << "["
-		<< sink->index() << "]: " << flags;
+	LOG(MediaDevice, Debug) << *link << ": " << flags;
 
 	return 0;
 }