[libcamera-devel] [PATCH 1/3] Early return if no cameras are found on the system.

[Kieran Bingham]

Hi Laurent,

On 24/11/2020 14:40, Laurent Pinchart wrote:
> Hi Kieran,
> 
> On Thu, Nov 19, 2020 at 12:40:08PM +0000, Kieran Bingham wrote:
>> Hi Umang,
>>
>> $SUBJECT is missing a "simple-cam:" prefix to match the others.
>>
>> Was this patch already posted somewhere?
>>
>> I don't seem to be able to find it, but I do half-recollect it ;-)
>> I just can't figure out where from.
>>
>> On 19/11/2020 12:34, Umang Jain wrote:
>>> Failing to do so, the codepath will segfault while trying to acquire
>>> a non-existent camera.
>>>
>>> Signed-off-by: Umang Jain <email at uajain.com>
>>> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>>> ---
>>>  simple-cam.cpp | 7 +++++++
>>>  1 file changed, 7 insertions(+)
>>>
>>> diff --git a/simple-cam.cpp b/simple-cam.cpp
>>> index 727bb6d..af9c8b1 100644
>>> --- a/simple-cam.cpp
>>> +++ b/simple-cam.cpp
>>> @@ -307,6 +307,13 @@ int main()
>>>  	for (std::unique_ptr<Request> &request : requests)
>>>  		camera->queueRequest(request.get());
>>>  
>>> +	if(!cm->cameras().size()) {
>>> +		std::cout << "No cameras were identified on the system."
>>> +			  << std::endl;
>>> +		cm->stop();
>>> +		return -1;
>>
>> I think we should we return -ENODEV instead of -1 here. We already use
>> errno values (for example -ENOMEM is used).
> 
> Note that the return value of the main() function is a bit special. C
> and C++ define EXIT_SUCCESS and EXIT_FAILURE to denote success and
> failure respectively (not a surprise :-)). The value of those macros is
> unspecified. See https://en.cppreference.com/w/c/program/EXIT_status.
> 
> (Interestingly, the standard defines that both EXIT_SUCCESS and the
> value zero indicate successful program execution status, but don't
> require EXIT_SUCCESS to be equal to zero.)
> 
> Other values are allowed, but only the 8 least significant bits are
> conveyed to the parent, so the range of usable values is effectively
> 0-255. Furthermore, shells may reserve some values. For instance, bash
> sets $? to 126 if a command is found but not executable, 127 if the
> command is not found, and 128+N if the command terminates with a fatal
> signal N.
> 
> As I don't expect we'll need to convey failure reasons through the exit
> status for simple cam, how about returning either 0 and 1, or
> EXIT_SUCCESS and EXIT_FAILURE ?

Aha - good point, I'd totally missed that. I guess I don't spend much
time in main() :-)

I'd prefer the explicit EXIT_SUCCESS / EXIT_FAILURE in these cases, but
it might be nice to make the file consistent either way.

I'll leave it up to you Umang to decide what you prefer.

--
Kieran


> 
>>> +	}
>>> +
>>>  	/*
>>>  	 * --------------------------------------------------------------------
>>>  	 * Run an EventLoop
> 

-- 
Regards
--
Kieran