[libcamera-devel] [PATCH v2 3/4] libcamera: utils: Use internal basename implementation.

[Kieran Bingham]

Hi Laurent,

On 01/04/2019 12:10, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Mon, Apr 01, 2019 at 06:03:14PM +0700, Kieran Bingham wrote:
>> Differing implementations of basename() exist, some of which may modify
>> the content of the string passed as an argument.
>>
>> The implementation of basename() is trivial, thus to support different
>> toolchains, provide our own version which accepts and returns a const
>> char*.
> 
> s/char\*/char */
> 
>>
>> Update the call sites to use the new implementation.
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>> ---
>>  src/libcamera/include/utils.h |  2 ++
>>  src/libcamera/log.cpp         |  2 +-
>>  src/libcamera/meson.build     |  1 +
>>  src/libcamera/utils.cpp       | 46 +++++++++++++++++++++++++++++++++++
>>  4 files changed, 50 insertions(+), 1 deletion(-)
>>  create mode 100644 src/libcamera/utils.cpp
>>
>> diff --git a/src/libcamera/include/utils.h b/src/libcamera/include/utils.h
>> index 73fa2e69b029..1b2a62c0fda7 100644
>> --- a/src/libcamera/include/utils.h
>> +++ b/src/libcamera/include/utils.h
>> @@ -15,6 +15,8 @@ namespace libcamera {
>>  
>>  namespace utils {
>>  
>> +const char *basename(const char *path);
>> +
>>  /* C++11 doesn't provide std::make_unique */
>>  template<typename T, typename... Args>
>>  std::unique_ptr<T> make_unique(Args&&... args)
>> diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp
>> index 26ebf410a7a9..eb444c31857d 100644
>> --- a/src/libcamera/log.cpp
>> +++ b/src/libcamera/log.cpp
>> @@ -438,7 +438,7 @@ void LogMessage::init(const char *fileName, unsigned int line)
>>  
>>  	msgStream_ << " " << log_severity_name(severity_);
>>  	msgStream_ << " " << category_.name();
>> -	msgStream_ << " " << basename(fileName) << ":" << line << " ";
>> +	msgStream_ << " " << utils::basename(fileName) << ":" << line << " ";
>>  }
>>  
>>  LogMessage::~LogMessage()
>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
>> index 8384cd0af451..863cb60d4b90 100644
>> --- a/src/libcamera/meson.build
>> +++ b/src/libcamera/meson.build
>> @@ -16,6 +16,7 @@ libcamera_sources = files([
>>      'signal.cpp',
>>      'stream.cpp',
>>      'timer.cpp',
>> +    'utils.cpp',
>>      'v4l2_device.cpp',
>>      'v4l2_subdevice.cpp',
>>  ])
>> diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp
>> new file mode 100644
>> index 000000000000..70936e36c5d5
>> --- /dev/null
>> +++ b/src/libcamera/utils.cpp
>> @@ -0,0 +1,46 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2019, Google Inc.
>> + *
>> + * utils.cpp - Miscellaneous utility functions
>> + */
>> +
>> +#include <string.h>
>> +
>> +#include "utils.h"
>> +
>> +/**
>> + * \file utils.h
>> + * \brief Miscellaneous utility functions
>> + */
>> +
>> +namespace libcamera {
>> +
>> +namespace utils {
>> +
>> +/**
>> + * \def ARRAY_SIZE(array)
>> + * \brief Determine the number of elemnents in the static array.
> 
> s/elemnents/elements/

Doh. Fixed.


> 
>> + */
>> +
>> +/**
>> + * \brief Strip the directory prefix from the path
> 
> Missing "\param[in] path" and \return.
> 
>> + *
>> + * basename is implemented differently across different toolchains.
>> + * Ensure consistency by providing our own implementation.
> 
> I would instead tell what behaviour we provide. For instance you could
> mention that this implementation matches the one provided by the GNU
> libc.
> 

Sure sounds good.


>> + */
>> +const char *basename(const char *path)
>> +{
>> +       const char *base = strrchr(path, '/');
>> +       return base ? base + 1 : path;
>> +}
>> +
>> +
>> +/**
>> + * \fn libcamera::utils::make_unique(Args &&... args)
>> + * \brief Construct an object and return a std::unique ptr to it
> 
> "Constructs an object of type T and wraps it in a std::unique_ptr." to
> match the documentation from
> https://en.cppreference.com/w/cpp/memory/unique_ptr/make_unique ?
> 

Updated.


>> + */
>> +
>> +} /* namespace utils */
>> +
>> +} /* namespace libcamera */
> 

-- 
Regards
--
Kieran