[libcamera-devel] [PATCH v2 1/2] test: ipc: unixsocket: Close open fds on error paths

[Kieran Bingham]

Hi Umang,

On 15/04/2020 16:04, Umang Jain wrote:
> Pointed out by Coverity DefectId=279052
> 
> Signed-off-by: Umang Jain <email at uajain.com>
> ---
>  test/ipc/unixsocket.cpp | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp
> index 5348f35..7bc6a89 100644
> --- a/test/ipc/unixsocket.cpp
> +++ b/test/ipc/unixsocket.cpp
> @@ -16,6 +16,7 @@
>  #include <unistd.h>
>  
>  #include <libcamera/event_dispatcher.h>
> +#include <libcamera/file_descriptor.h>
>  #include <libcamera/timer.h>
>  
>  #include "ipc_unixsocket.h"
> @@ -467,18 +468,22 @@ private:
>  		if (fd < 0)
>  			return fd;
>  
> +		FileDescriptor file_desc = FileDescriptor(fd);
> +		close(fd);

Since you created this patch, the FileDescriptor has been updated to
take move semantics to avoid having to close an FD directly after
instantiating.

I think updating this patch to use that new API needs to happen in this
patch, could you update please?

I'm not 100% sure of the syntax, but I think it's something like as
simple as:

file_desc = FileDescriptor(std::move(fd));


> +		if (file_desc.fd() < 0)
> +			return file_desc.fd();
> +
>  		int size = 0;
>  		for (unsigned int i = 0; i < num; i++) {
> -			int clone = dup(fd);
> -			if (clone < 0)
> -				return clone;
> +			FileDescriptor *clone = new FileDescriptor(file_desc.fd());
> +			int clone_int = clone->fd();

I wouldn't use a local var here,, just reference clone->fd() below instead.

> +			if (clone_int < 0)
> +				return clone_int;
>  
> -			size += calculateLength(clone);
> -			message->fds.push_back(clone);
> +			size += calculateLength(clone_int);
> +			message->fds.push_back(clone_int);
>  		}
>  
> -		close(fd);
> -
>  		return size;
>  	}
>  
> 

-- 
Regards
--
Kieran