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

[Kieran Bingham]

Hi Laurent,

On 13/04/2020 13:34, Laurent Pinchart wrote:
> Hi Umang,
> 
> Thank you for the patch.
> 
> On Mon, Apr 13, 2020 at 10:46:51AM +0000, Umang Jain wrote:
>> Pointed out by Coverity DefectId=279099
>>
>> Signed-off-by: Umang Jain <email at uajain.com>
>> ---
>>  test/ipc/unixsocket.cpp | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp
>> index f53042b..5348f35 100644
>> --- a/test/ipc/unixsocket.cpp
>> +++ b/test/ipc/unixsocket.cpp
>> @@ -145,6 +145,7 @@ private:
>>  
>>  					if (num < 0) {
>>  						cerr << "Read failed" << endl;
>> +						close(outfd);
>>  						stop(-EIO);
>>  						return;
>>  					} else if (!num)
>> @@ -152,6 +153,7 @@ private:
>>  
>>  					if (write(outfd, buf, num) < 0) {
>>  						cerr << "Write failed" << endl;
>> +						close(outfd);
>>  						stop(-EIO);
>>  						return;
>>  					}
> 
> I wonder if we should extend the File class to support read and write,
> and use it here to simplify the cleanup paths, but that's probably a bit
> too much just to fix this test, so

For 'now' I agree - that would then be extending our libcamera framework
classes just to provide test functionality, however once we start
needing to read/write to files for ... say configuration files - I can
see we might well want to extend our File class.

At that point - (after that's done) it might well be worth converting
any file accesses to use our helper class to bring in all the RAII
benefits and wrappings.

But for this bug..

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

Though I see the File class didn't exist when this patch was posted
either - but it's in now :-)

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

-- 
Regards
--
Kieran