[libcamera-devel] [PATCH] libcamera: v4l2_device: Return const std::string

[Kieran Bingham]

Hi Jacopo,

On 21/01/2019 09:26, Laurent Pinchart wrote:
> Hi Jacopo,
> 
> On Mon, Jan 21, 2019 at 10:20:20AM +0100, Jacopo Mondi wrote:
>> Make getter functions of V4L2Device return std::string as the rest of
>> the library does.
>
> This is something that has been discussed during the review of Kieran's
> original patch I believe. The question was whether it would be more
> efficient to use const char * or a an std::string, and the answer to
> that question depends on the expected usage of those methods. If all
> users need an std::string, then I agree that the methods below can as
> well construct one. On the other hand, if some users need a const char
> *, it is a bit pointless to convert from const char * to std::string and
> then back to const char * using c_str(). This is why we have opted for
> const char * here.


And because the compiler can automatically convert from a const char *
to a std::string using the string constructors ... so you can still use
these methods anywhere you would use a std::string...




> This is a generic question that we should ask ourselves through the
> library. Anyone with free time to analyze the problem ? :-)
> 
>> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
>> ---
>>  src/libcamera/include/v4l2_device.h | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h
>> index 474c05b..5fe3d82 100644
>> --- a/src/libcamera/include/v4l2_device.h
>> +++ b/src/libcamera/include/v4l2_device.h
>> @@ -45,9 +45,9 @@ public:
>>  	bool isOpen() const;
>>  	void close();
>>
>> -	const char *driverName() const { return caps_.driver(); }
>> -	const char *deviceName() const { return caps_.card(); }
>> -	const char *busName() const { return caps_.bus_info(); }
>> +	const std::string driverName() const { return std::string(caps_.driver()); }
>> +	const std::string deviceName() const { return std::string(caps_.card()); }
>> +	const std::string busName() const { return std::string(caps_.bus_info()); }
> 
> If we wanted to return an std::string, we should drop the const as the
> string is returned by value.
> 
>>
>>  private:
>>  	std::string devnode_;
> 

-- 
Regards
--
Kieran