[libcamera-devel,v3,07/17] py: Set EFD_CLOEXEC on eventfd to avoid fd leaking
diff mbox series

Message ID 20220701084521.31831-8-tomi.valkeinen@ideasonboard.com
State Accepted
Headers show
Series
  • Python bindings event handling
Related show

Commit Message

Tomi Valkeinen July 1, 2022, 8:45 a.m. UTC 
Set EFD_CLOEXEC on eventfd to avoid fd leaking.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 src/py/libcamera/py_camera_manager.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jacopo Mondi Aug. 18, 2022, 2:25 p.m. UTC | #1 
Hi Tomi

On Fri, Jul 01, 2022 at 11:45:11AM +0300, Tomi Valkeinen wrote:
> Set EFD_CLOEXEC on eventfd to avoid fd leaking.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  src/py/libcamera/py_camera_manager.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/py/libcamera/py_camera_manager.cpp b/src/py/libcamera/py_camera_manager.cpp
> index 51a890c8..3d422c9e 100644
> --- a/src/py/libcamera/py_camera_manager.cpp
> +++ b/src/py/libcamera/py_camera_manager.cpp
> @@ -22,7 +22,7 @@ PyCameraManager::PyCameraManager()
>
>  	cameraManager_ = std::make_unique<CameraManager>();
>
> -	int fd = eventfd(0, 0);
> +	int fd = eventfd(0, EFD_CLOEXEC);

Or you could

        eventfd_ = UniqueFd(eventfd((0, EFD_CLOEXEC));
        if (!eventfd_.isValid())

And squash with the previous ones

>  	if (fd == -1)
>  		throw std::system_error(errno, std::generic_category(),
>  					"Failed to create eventfd");
> --
> 2.34.1
>
Tomi Valkeinen Aug. 18, 2022, 2:52 p.m. UTC | #2 
On 18/08/2022 17:25, Jacopo Mondi wrote:
> Hi Tomi
> 
> On Fri, Jul 01, 2022 at 11:45:11AM +0300, Tomi Valkeinen wrote:
>> Set EFD_CLOEXEC on eventfd to avoid fd leaking.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>> ---
>>   src/py/libcamera/py_camera_manager.cpp | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/py/libcamera/py_camera_manager.cpp b/src/py/libcamera/py_camera_manager.cpp
>> index 51a890c8..3d422c9e 100644
>> --- a/src/py/libcamera/py_camera_manager.cpp
>> +++ b/src/py/libcamera/py_camera_manager.cpp
>> @@ -22,7 +22,7 @@ PyCameraManager::PyCameraManager()
>>
>>   	cameraManager_ = std::make_unique<CameraManager>();
>>
>> -	int fd = eventfd(0, 0);
>> +	int fd = eventfd(0, EFD_CLOEXEC);
> 
> Or you could
> 
>          eventfd_ = UniqueFd(eventfd((0, EFD_CLOEXEC));
>          if (!eventfd_.isValid())

Yes... But that hides the eventfd() a bit there. And makes the line 
longer, especially after we add the EFD_NONBLOCK. Nothing big, but I do 
like the current style better.

  Tomi
Laurent Pinchart Aug. 18, 2022, 3:37 p.m. UTC | #3 
Hi Tomi,

Thank you for the patch.

On Fri, Jul 01, 2022 at 11:45:11AM +0300, Tomi Valkeinen wrote:
> Set EFD_CLOEXEC on eventfd to avoid fd leaking.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

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

> ---
>  src/py/libcamera/py_camera_manager.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/py/libcamera/py_camera_manager.cpp b/src/py/libcamera/py_camera_manager.cpp
> index 51a890c8..3d422c9e 100644
> --- a/src/py/libcamera/py_camera_manager.cpp
> +++ b/src/py/libcamera/py_camera_manager.cpp
> @@ -22,7 +22,7 @@ PyCameraManager::PyCameraManager()
>  
>  	cameraManager_ = std::make_unique<CameraManager>();
>  
> -	int fd = eventfd(0, 0);
> +	int fd = eventfd(0, EFD_CLOEXEC);
>  	if (fd == -1)
>  		throw std::system_error(errno, std::generic_category(),
>  					"Failed to create eventfd");

Patch
diff mbox series 

diff --git a/src/py/libcamera/py_camera_manager.cpp b/src/py/libcamera/py_camera_manager.cpp
index 51a890c8..3d422c9e 100644
--- a/src/py/libcamera/py_camera_manager.cpp
+++ b/src/py/libcamera/py_camera_manager.cpp
@@ -22,7 +22,7 @@  PyCameraManager::PyCameraManager()
 
 	cameraManager_ = std::make_unique<CameraManager>();
 
-	int fd = eventfd(0, 0);
+	int fd = eventfd(0, EFD_CLOEXEC);
 	if (fd == -1)
 		throw std::system_error(errno, std::generic_category(),
 					"Failed to create eventfd");