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

[Laurent Pinchart]

Hi Kieran,

On Mon, Jan 21, 2019 at 09:34:42AM +0000, Kieran Bingham wrote:
> On 21/01/2019 09:26, Laurent Pinchart wrote:
> > 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...

Note that in that case the std::string constructor will be inlined,
which can be an issue if the function returning const char * is called
from a very large number of places. In this specific case the functions
are themselves inlined anyway, so it would make very little difference.

> > 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,

Laurent Pinchart