[libcamera-devel,1/3] libcamera: utils: Add string splitter utility function
diff mbox series

Message ID 20200213130908.23638-2-kieran.bingham@ideasonboard.com
State Accepted
Headers show
Series
  • utils: Provide string splitting
Related show

Commit Message

Kieran Bingham Feb. 13, 2020, 1:09 p.m. UTC 
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Add a utils::split() function that splits a string for the purpose of
iterating over substrings. It returns an object of unspecified type that
can be used in range-based for loops.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 Documentation/Doxyfile.in     |  1 +
 src/libcamera/include/utils.h | 34 +++++++++++++++++++
 src/libcamera/utils.cpp       | 62 +++++++++++++++++++++++++++++++++++
 3 files changed, 97 insertions(+)

Comments

Kieran Bingham Feb. 13, 2020, 1:24 p.m. UTC | #1 
Hi Laurent,

On 13/02/2020 13:09, Kieran Bingham wrote:
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Add a utils::split() function that splits a string for the purpose of
> iterating over substrings. It returns an object of unspecified type that
> can be used in range-based for loops.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

This is really helpful, thanks

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

> ---
>  Documentation/Doxyfile.in     |  1 +
>  src/libcamera/include/utils.h | 34 +++++++++++++++++++
>  src/libcamera/utils.cpp       | 62 +++++++++++++++++++++++++++++++++++
>  3 files changed, 97 insertions(+)
> 
> diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in
> index 1c46b04b3f7e..beeaf6d3cf48 100644
> --- a/Documentation/Doxyfile.in
> +++ b/Documentation/Doxyfile.in
> @@ -880,6 +880,7 @@ EXCLUDE_SYMBOLS        = libcamera::BoundMethodArgs \
>                           libcamera::BoundMethodStatic \
>                           libcamera::SignalBase \
>                           libcamera::*::Private \
> +                         libcamera::*::details::* \
>                           std::*
>  
>  # The EXAMPLE_PATH tag can be used to specify one or more files or directories
> diff --git a/src/libcamera/include/utils.h b/src/libcamera/include/utils.h
> index e467eb21c518..080ea6614de0 100644
> --- a/src/libcamera/include/utils.h
> +++ b/src/libcamera/include/utils.h
> @@ -108,6 +108,40 @@ inline _hex hex<uint64_t>(uint64_t value, unsigned int width)
>  
>  size_t strlcpy(char *dst, const char *src, size_t size);
>  
> +namespace details {
> +
> +class StringSplitter
> +{
> +public:
> +	StringSplitter(const std::string &str, const std::string &delim);
> +
> +	class iterator
> +	{
> +	public:
> +		iterator(const StringSplitter *ss, std::string::size_type pos);
> +
> +		iterator &operator++();
> +		std::string operator*() const;
> +		bool operator!=(const iterator &other) const;
> +
> +	private:
> +		const StringSplitter *ss_;
> +		std::string::size_type pos_;
> +		std::string::size_type next_;
> +	};
> +
> +	iterator begin() const;
> +	iterator end() const;
> +
> +private:
> +	std::string str_;
> +	std::string delim_;
> +};
> +
> +} /* namespace details */
> +
> +details::StringSplitter split(const std::string &str, const std::string &delim);
> +
>  } /* namespace utils */
>  
>  } /* namespace libcamera */
> diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp
> index 4beffdab5eb6..fe027c0b009c 100644
> --- a/src/libcamera/utils.cpp
> +++ b/src/libcamera/utils.cpp
> @@ -199,6 +199,68 @@ size_t strlcpy(char *dst, const char *src, size_t size)
>  	return strlen(src);
>  }
>  
> +details::StringSplitter::StringSplitter(const std::string &str, const std::string &delim)
> +	: str_(str), delim_(delim)
> +{
> +}
> +
> +details::StringSplitter::iterator::iterator(const details::StringSplitter *ss, std::string::size_type pos)
> +	: ss_(ss), pos_(pos)
> +{
> +	next_ = ss_->str_.find(ss_->delim_, pos_);
> +}
> +
> +details::StringSplitter::iterator &details::StringSplitter::iterator::operator++()
> +{
> +	pos_ = next_;
> +	if (pos_ != std::string::npos) {
> +		pos_ += ss_->delim_.length();
> +		next_ = ss_->str_.find(ss_->delim_, pos_);
> +	}
> +
> +	return *this;
> +}
> +
> +std::string details::StringSplitter::iterator::operator*() const
> +{
> +	std::string::size_type count;
> +	count = next_ != std::string::npos ? next_ - pos_ : next_;
> +	return ss_->str_.substr(pos_, count);
> +}
> +
> +bool details::StringSplitter::iterator::operator!=(const details::StringSplitter::iterator &other) const
> +{
> +	return pos_ != other.pos_;
> +}
> +
> +details::StringSplitter::iterator details::StringSplitter::begin() const
> +{
> +	return iterator(this, 0);
> +}
> +
> +details::StringSplitter::iterator details::StringSplitter::end() const
> +{
> +	return iterator(this, std::string::npos);
> +}
> +
> +/**
> + * \fn split(const std::string &str, const std::string &delim)
> + * \brief Split a string based on a delimiter
> + * \param[in] str The string to split
> + * \param[in] delim The delimiter string
> + *
> + * This function splits the string \a str into substrings based on the
> + * delimiter \a delim. It returns an object of unspecified type that can be
> + * used in a range-based for loop and yields the substrings in sequence.
> + *
> + * \return An object that can be used in a range-based for loop to iterate over
> + * the substrings
> + */
> +details::StringSplitter split(const std::string &str, const std::string &delim)
> +{
> +	return details::StringSplitter(str, delim);
> +}
> +
>  } /* namespace utils */
>  
>  } /* namespace libcamera */
>
Laurent Pinchart Feb. 13, 2020, 1:36 p.m. UTC | #2 
Hi Kieran,

On Thu, Feb 13, 2020 at 01:24:27PM +0000, Kieran Bingham wrote:
> On 13/02/2020 13:09, Kieran Bingham wrote:
> > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > Add a utils::split() function that splits a string for the purpose of
> > iterating over substrings. It returns an object of unspecified type that
> > can be used in range-based for loops.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> This is really helpful, thanks
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> > ---
> >  Documentation/Doxyfile.in     |  1 +
> >  src/libcamera/include/utils.h | 34 +++++++++++++++++++
> >  src/libcamera/utils.cpp       | 62 +++++++++++++++++++++++++++++++++++
> >  3 files changed, 97 insertions(+)
> > 
> > diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in
> > index 1c46b04b3f7e..beeaf6d3cf48 100644
> > --- a/Documentation/Doxyfile.in
> > +++ b/Documentation/Doxyfile.in
> > @@ -880,6 +880,7 @@ EXCLUDE_SYMBOLS        = libcamera::BoundMethodArgs \
> >                           libcamera::BoundMethodStatic \
> >                           libcamera::SignalBase \
> >                           libcamera::*::Private \
> > +                         libcamera::*::details::* \
> >                           std::*
> >  
> >  # The EXAMPLE_PATH tag can be used to specify one or more files or directories
> > diff --git a/src/libcamera/include/utils.h b/src/libcamera/include/utils.h
> > index e467eb21c518..080ea6614de0 100644
> > --- a/src/libcamera/include/utils.h
> > +++ b/src/libcamera/include/utils.h
> > @@ -108,6 +108,40 @@ inline _hex hex<uint64_t>(uint64_t value, unsigned int width)
> >  
> >  size_t strlcpy(char *dst, const char *src, size_t size);
> >  
> > +namespace details {
> > +
> > +class StringSplitter
> > +{
> > +public:
> > +	StringSplitter(const std::string &str, const std::string &delim);
> > +
> > +	class iterator
> > +	{
> > +	public:
> > +		iterator(const StringSplitter *ss, std::string::size_type pos);
> > +
> > +		iterator &operator++();
> > +		std::string operator*() const;
> > +		bool operator!=(const iterator &other) const;
> > +
> > +	private:
> > +		const StringSplitter *ss_;
> > +		std::string::size_type pos_;
> > +		std::string::size_type next_;
> > +	};
> > +
> > +	iterator begin() const;
> > +	iterator end() const;
> > +
> > +private:
> > +	std::string str_;
> > +	std::string delim_;

Note that I would really like if we could store references to str and
delim here, but my attempt to do so resulted in problems, I believe due
to the values passed to the StringSplitter constructor being
temporaries. I don't know if there's a good way to solve that.

> > +};
> > +
> > +} /* namespace details */
> > +
> > +details::StringSplitter split(const std::string &str, const std::string &delim);
> > +
> >  } /* namespace utils */
> >  
> >  } /* namespace libcamera */
> > diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp
> > index 4beffdab5eb6..fe027c0b009c 100644
> > --- a/src/libcamera/utils.cpp
> > +++ b/src/libcamera/utils.cpp
> > @@ -199,6 +199,68 @@ size_t strlcpy(char *dst, const char *src, size_t size)
> >  	return strlen(src);
> >  }
> >  
> > +details::StringSplitter::StringSplitter(const std::string &str, const std::string &delim)
> > +	: str_(str), delim_(delim)
> > +{
> > +}
> > +
> > +details::StringSplitter::iterator::iterator(const details::StringSplitter *ss, std::string::size_type pos)
> > +	: ss_(ss), pos_(pos)
> > +{
> > +	next_ = ss_->str_.find(ss_->delim_, pos_);
> > +}
> > +
> > +details::StringSplitter::iterator &details::StringSplitter::iterator::operator++()
> > +{
> > +	pos_ = next_;
> > +	if (pos_ != std::string::npos) {
> > +		pos_ += ss_->delim_.length();
> > +		next_ = ss_->str_.find(ss_->delim_, pos_);
> > +	}
> > +
> > +	return *this;
> > +}
> > +
> > +std::string details::StringSplitter::iterator::operator*() const
> > +{
> > +	std::string::size_type count;
> > +	count = next_ != std::string::npos ? next_ - pos_ : next_;
> > +	return ss_->str_.substr(pos_, count);
> > +}
> > +
> > +bool details::StringSplitter::iterator::operator!=(const details::StringSplitter::iterator &other) const
> > +{
> > +	return pos_ != other.pos_;
> > +}
> > +
> > +details::StringSplitter::iterator details::StringSplitter::begin() const
> > +{
> > +	return iterator(this, 0);
> > +}
> > +
> > +details::StringSplitter::iterator details::StringSplitter::end() const
> > +{
> > +	return iterator(this, std::string::npos);
> > +}
> > +
> > +/**
> > + * \fn split(const std::string &str, const std::string &delim)
> > + * \brief Split a string based on a delimiter
> > + * \param[in] str The string to split
> > + * \param[in] delim The delimiter string
> > + *
> > + * This function splits the string \a str into substrings based on the
> > + * delimiter \a delim. It returns an object of unspecified type that can be
> > + * used in a range-based for loop and yields the substrings in sequence.
> > + *
> > + * \return An object that can be used in a range-based for loop to iterate over
> > + * the substrings
> > + */
> > +details::StringSplitter split(const std::string &str, const std::string &delim)
> > +{
> > +	return details::StringSplitter(str, delim);
> > +}
> > +
> >  } /* namespace utils */
> >  
> >  } /* namespace libcamera */
Kieran Bingham Feb. 13, 2020, 4:10 p.m. UTC | #3 
Hi Laurent,

On 13/02/2020 13:36, Laurent Pinchart wrote:
> Hi Kieran,
> 
> On Thu, Feb 13, 2020 at 01:24:27PM +0000, Kieran Bingham wrote:
>> On 13/02/2020 13:09, Kieran Bingham wrote:
>>> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>
>>> Add a utils::split() function that splits a string for the purpose of
>>> iterating over substrings. It returns an object of unspecified type that
>>> can be used in range-based for loops.
>>>
>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>
>> This is really helpful, thanks
>>
>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>>
>>> ---
>>>  Documentation/Doxyfile.in     |  1 +
>>>  src/libcamera/include/utils.h | 34 +++++++++++++++++++
>>>  src/libcamera/utils.cpp       | 62 +++++++++++++++++++++++++++++++++++
>>>  3 files changed, 97 insertions(+)
>>>
>>> diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in
>>> index 1c46b04b3f7e..beeaf6d3cf48 100644
>>> --- a/Documentation/Doxyfile.in
>>> +++ b/Documentation/Doxyfile.in
>>> @@ -880,6 +880,7 @@ EXCLUDE_SYMBOLS        = libcamera::BoundMethodArgs \
>>>                           libcamera::BoundMethodStatic \
>>>                           libcamera::SignalBase \
>>>                           libcamera::*::Private \
>>> +                         libcamera::*::details::* \
>>>                           std::*
>>>  
>>>  # The EXAMPLE_PATH tag can be used to specify one or more files or directories
>>> diff --git a/src/libcamera/include/utils.h b/src/libcamera/include/utils.h
>>> index e467eb21c518..080ea6614de0 100644
>>> --- a/src/libcamera/include/utils.h
>>> +++ b/src/libcamera/include/utils.h
>>> @@ -108,6 +108,40 @@ inline _hex hex<uint64_t>(uint64_t value, unsigned int width)
>>>  
>>>  size_t strlcpy(char *dst, const char *src, size_t size);
>>>  
>>> +namespace details {
>>> +
>>> +class StringSplitter
>>> +{
>>> +public:
>>> +	StringSplitter(const std::string &str, const std::string &delim);
>>> +
>>> +	class iterator
>>> +	{
>>> +	public:
>>> +		iterator(const StringSplitter *ss, std::string::size_type pos);
>>> +
>>> +		iterator &operator++();
>>> +		std::string operator*() const;
>>> +		bool operator!=(const iterator &other) const;
>>> +
>>> +	private:
>>> +		const StringSplitter *ss_;
>>> +		std::string::size_type pos_;
>>> +		std::string::size_type next_;
>>> +	};
>>> +
>>> +	iterator begin() const;
>>> +	iterator end() const;
>>> +
>>> +private:
>>> +	std::string str_;
>>> +	std::string delim_;
> 
> Note that I would really like if we could store references to str and
> delim here, but my attempt to do so resulted in problems, I believe due
> to the values passed to the StringSplitter constructor being
> temporaries. I don't know if there's a good way to solve that.

This isn't used in any hot-path, so I'm not worried for now.

If it comes up in the future it can be investigated then.

Would you like to add a todo: ? or not worth it?


>>> +};
>>> +
>>> +} /* namespace details */
>>> +
>>> +details::StringSplitter split(const std::string &str, const std::string &delim);
>>> +
>>>  } /* namespace utils */
>>>  
>>>  } /* namespace libcamera */
>>> diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp
>>> index 4beffdab5eb6..fe027c0b009c 100644
>>> --- a/src/libcamera/utils.cpp
>>> +++ b/src/libcamera/utils.cpp
>>> @@ -199,6 +199,68 @@ size_t strlcpy(char *dst, const char *src, size_t size)
>>>  	return strlen(src);
>>>  }
>>>  
>>> +details::StringSplitter::StringSplitter(const std::string &str, const std::string &delim)
>>> +	: str_(str), delim_(delim)
>>> +{
>>> +}
>>> +
>>> +details::StringSplitter::iterator::iterator(const details::StringSplitter *ss, std::string::size_type pos)
>>> +	: ss_(ss), pos_(pos)
>>> +{
>>> +	next_ = ss_->str_.find(ss_->delim_, pos_);
>>> +}
>>> +
>>> +details::StringSplitter::iterator &details::StringSplitter::iterator::operator++()
>>> +{
>>> +	pos_ = next_;
>>> +	if (pos_ != std::string::npos) {
>>> +		pos_ += ss_->delim_.length();
>>> +		next_ = ss_->str_.find(ss_->delim_, pos_);
>>> +	}
>>> +
>>> +	return *this;
>>> +}
>>> +
>>> +std::string details::StringSplitter::iterator::operator*() const
>>> +{
>>> +	std::string::size_type count;
>>> +	count = next_ != std::string::npos ? next_ - pos_ : next_;
>>> +	return ss_->str_.substr(pos_, count);
>>> +}
>>> +
>>> +bool details::StringSplitter::iterator::operator!=(const details::StringSplitter::iterator &other) const
>>> +{
>>> +	return pos_ != other.pos_;
>>> +}
>>> +
>>> +details::StringSplitter::iterator details::StringSplitter::begin() const
>>> +{
>>> +	return iterator(this, 0);
>>> +}
>>> +
>>> +details::StringSplitter::iterator details::StringSplitter::end() const
>>> +{
>>> +	return iterator(this, std::string::npos);
>>> +}
>>> +
>>> +/**
>>> + * \fn split(const std::string &str, const std::string &delim)
>>> + * \brief Split a string based on a delimiter
>>> + * \param[in] str The string to split
>>> + * \param[in] delim The delimiter string
>>> + *
>>> + * This function splits the string \a str into substrings based on the
>>> + * delimiter \a delim. It returns an object of unspecified type that can be
>>> + * used in a range-based for loop and yields the substrings in sequence.
>>> + *
>>> + * \return An object that can be used in a range-based for loop to iterate over
>>> + * the substrings
>>> + */
>>> +details::StringSplitter split(const std::string &str, const std::string &delim)
>>> +{
>>> +	return details::StringSplitter(str, delim);
>>> +}
>>> +
>>>  } /* namespace utils */
>>>  
>>>  } /* namespace libcamera */
>
Laurent Pinchart Feb. 13, 2020, 4:24 p.m. UTC | #4 
Hi Kieran,

On Thu, Feb 13, 2020 at 04:10:10PM +0000, Kieran Bingham wrote:
> On 13/02/2020 13:36, Laurent Pinchart wrote:
> > On Thu, Feb 13, 2020 at 01:24:27PM +0000, Kieran Bingham wrote:
> >> On 13/02/2020 13:09, Kieran Bingham wrote:
> >>> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>
> >>> Add a utils::split() function that splits a string for the purpose of
> >>> iterating over substrings. It returns an object of unspecified type that
> >>> can be used in range-based for loops.
> >>>
> >>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>
> >> This is really helpful, thanks
> >>
> >> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> >>
> >>> ---
> >>>  Documentation/Doxyfile.in     |  1 +
> >>>  src/libcamera/include/utils.h | 34 +++++++++++++++++++
> >>>  src/libcamera/utils.cpp       | 62 +++++++++++++++++++++++++++++++++++
> >>>  3 files changed, 97 insertions(+)
> >>>
> >>> diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in
> >>> index 1c46b04b3f7e..beeaf6d3cf48 100644
> >>> --- a/Documentation/Doxyfile.in
> >>> +++ b/Documentation/Doxyfile.in
> >>> @@ -880,6 +880,7 @@ EXCLUDE_SYMBOLS        = libcamera::BoundMethodArgs \
> >>>                           libcamera::BoundMethodStatic \
> >>>                           libcamera::SignalBase \
> >>>                           libcamera::*::Private \
> >>> +                         libcamera::*::details::* \
> >>>                           std::*
> >>>  
> >>>  # The EXAMPLE_PATH tag can be used to specify one or more files or directories
> >>> diff --git a/src/libcamera/include/utils.h b/src/libcamera/include/utils.h
> >>> index e467eb21c518..080ea6614de0 100644
> >>> --- a/src/libcamera/include/utils.h
> >>> +++ b/src/libcamera/include/utils.h
> >>> @@ -108,6 +108,40 @@ inline _hex hex<uint64_t>(uint64_t value, unsigned int width)
> >>>  
> >>>  size_t strlcpy(char *dst, const char *src, size_t size);
> >>>  
> >>> +namespace details {
> >>> +
> >>> +class StringSplitter
> >>> +{
> >>> +public:
> >>> +	StringSplitter(const std::string &str, const std::string &delim);
> >>> +
> >>> +	class iterator
> >>> +	{
> >>> +	public:
> >>> +		iterator(const StringSplitter *ss, std::string::size_type pos);
> >>> +
> >>> +		iterator &operator++();
> >>> +		std::string operator*() const;
> >>> +		bool operator!=(const iterator &other) const;
> >>> +
> >>> +	private:
> >>> +		const StringSplitter *ss_;
> >>> +		std::string::size_type pos_;
> >>> +		std::string::size_type next_;
> >>> +	};
> >>> +
> >>> +	iterator begin() const;
> >>> +	iterator end() const;
> >>> +
> >>> +private:
> >>> +	std::string str_;
> >>> +	std::string delim_;
> > 
> > Note that I would really like if we could store references to str and
> > delim here, but my attempt to do so resulted in problems, I believe due
> > to the values passed to the StringSplitter constructor being
> > temporaries. I don't know if there's a good way to solve that.
> 
> This isn't used in any hot-path, so I'm not worried for now.
> 
> If it comes up in the future it can be investigated then.
> 
> Would you like to add a todo: ? or not worth it?

It's always good to keep todos in mind :-) Please see below.

> >>> +};
> >>> +
> >>> +} /* namespace details */
> >>> +
> >>> +details::StringSplitter split(const std::string &str, const std::string &delim);
> >>> +
> >>>  } /* namespace utils */
> >>>  
> >>>  } /* namespace libcamera */
> >>> diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp
> >>> index 4beffdab5eb6..fe027c0b009c 100644
> >>> --- a/src/libcamera/utils.cpp
> >>> +++ b/src/libcamera/utils.cpp
> >>> @@ -199,6 +199,68 @@ size_t strlcpy(char *dst, const char *src, size_t size)
> >>>  	return strlen(src);
> >>>  }
> >>>  
> >>> +details::StringSplitter::StringSplitter(const std::string &str, const std::string &delim)
> >>> +	: str_(str), delim_(delim)
> >>> +{
> >>> +}
> >>> +
> >>> +details::StringSplitter::iterator::iterator(const details::StringSplitter *ss, std::string::size_type pos)
> >>> +	: ss_(ss), pos_(pos)
> >>> +{
> >>> +	next_ = ss_->str_.find(ss_->delim_, pos_);
> >>> +}
> >>> +
> >>> +details::StringSplitter::iterator &details::StringSplitter::iterator::operator++()
> >>> +{
> >>> +	pos_ = next_;
> >>> +	if (pos_ != std::string::npos) {
> >>> +		pos_ += ss_->delim_.length();
> >>> +		next_ = ss_->str_.find(ss_->delim_, pos_);
> >>> +	}
> >>> +
> >>> +	return *this;
> >>> +}
> >>> +
> >>> +std::string details::StringSplitter::iterator::operator*() const
> >>> +{
> >>> +	std::string::size_type count;
> >>> +	count = next_ != std::string::npos ? next_ - pos_ : next_;
> >>> +	return ss_->str_.substr(pos_, count);
> >>> +}
> >>> +
> >>> +bool details::StringSplitter::iterator::operator!=(const details::StringSplitter::iterator &other) const
> >>> +{
> >>> +	return pos_ != other.pos_;
> >>> +}
> >>> +
> >>> +details::StringSplitter::iterator details::StringSplitter::begin() const
> >>> +{
> >>> +	return iterator(this, 0);
> >>> +}
> >>> +
> >>> +details::StringSplitter::iterator details::StringSplitter::end() const
> >>> +{
> >>> +	return iterator(this, std::string::npos);
> >>> +}
> >>> +
> >>> +/**
> >>> + * \fn split(const std::string &str, const std::string &delim)
> >>> + * \brief Split a string based on a delimiter
> >>> + * \param[in] str The string to split
> >>> + * \param[in] delim The delimiter string
> >>> + *
> >>> + * This function splits the string \a str into substrings based on the
> >>> + * delimiter \a delim. It returns an object of unspecified type that can be
> >>> + * used in a range-based for loop and yields the substrings in sequence.
> >>> + *
> >>> + * \return An object that can be used in a range-based for loop to iterate over
> >>> + * the substrings
> >>> + */
> >>> +details::StringSplitter split(const std::string &str, const std::string &delim)
> >>> +{

	/** \todo Try to avoid copies of str and delim */

> >>> +	return details::StringSplitter(str, delim);
> >>> +}
> >>> +
> >>>  } /* namespace utils */
> >>>  
> >>>  } /* namespace libcamera */
Kieran Bingham Feb. 13, 2020, 4:32 p.m. UTC | #5 
Hi Laurent,

On 13/02/2020 16:24, Laurent Pinchart wrote:
> Hi Kieran,
> 
> On Thu, Feb 13, 2020 at 04:10:10PM +0000, Kieran Bingham wrote:
>> On 13/02/2020 13:36, Laurent Pinchart wrote:
>>> On Thu, Feb 13, 2020 at 01:24:27PM +0000, Kieran Bingham wrote:
>>>> On 13/02/2020 13:09, Kieran Bingham wrote:
>>>>> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>>>
>>>>> Add a utils::split() function that splits a string for the purpose of
>>>>> iterating over substrings. It returns an object of unspecified type that
>>>>> can be used in range-based for loops.
>>>>>
>>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>>
>>>> This is really helpful, thanks
>>>>
>>>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>>>>
>>>>> ---
>>>>>  Documentation/Doxyfile.in     |  1 +
>>>>>  src/libcamera/include/utils.h | 34 +++++++++++++++++++
>>>>>  src/libcamera/utils.cpp       | 62 +++++++++++++++++++++++++++++++++++
>>>>>  3 files changed, 97 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in
>>>>> index 1c46b04b3f7e..beeaf6d3cf48 100644
>>>>> --- a/Documentation/Doxyfile.in
>>>>> +++ b/Documentation/Doxyfile.in
>>>>> @@ -880,6 +880,7 @@ EXCLUDE_SYMBOLS        = libcamera::BoundMethodArgs \
>>>>>                           libcamera::BoundMethodStatic \
>>>>>                           libcamera::SignalBase \
>>>>>                           libcamera::*::Private \
>>>>> +                         libcamera::*::details::* \
>>>>>                           std::*
>>>>>  
>>>>>  # The EXAMPLE_PATH tag can be used to specify one or more files or directories
>>>>> diff --git a/src/libcamera/include/utils.h b/src/libcamera/include/utils.h
>>>>> index e467eb21c518..080ea6614de0 100644
>>>>> --- a/src/libcamera/include/utils.h
>>>>> +++ b/src/libcamera/include/utils.h
>>>>> @@ -108,6 +108,40 @@ inline _hex hex<uint64_t>(uint64_t value, unsigned int width)
>>>>>  
>>>>>  size_t strlcpy(char *dst, const char *src, size_t size);
>>>>>  
>>>>> +namespace details {
>>>>> +
>>>>> +class StringSplitter
>>>>> +{
>>>>> +public:
>>>>> +	StringSplitter(const std::string &str, const std::string &delim);
>>>>> +
>>>>> +	class iterator
>>>>> +	{
>>>>> +	public:
>>>>> +		iterator(const StringSplitter *ss, std::string::size_type pos);
>>>>> +
>>>>> +		iterator &operator++();
>>>>> +		std::string operator*() const;
>>>>> +		bool operator!=(const iterator &other) const;
>>>>> +
>>>>> +	private:
>>>>> +		const StringSplitter *ss_;
>>>>> +		std::string::size_type pos_;
>>>>> +		std::string::size_type next_;
>>>>> +	};
>>>>> +
>>>>> +	iterator begin() const;
>>>>> +	iterator end() const;
>>>>> +
>>>>> +private:
>>>>> +	std::string str_;
>>>>> +	std::string delim_;
>>>
>>> Note that I would really like if we could store references to str and
>>> delim here, but my attempt to do so resulted in problems, I believe due
>>> to the values passed to the StringSplitter constructor being
>>> temporaries. I don't know if there's a good way to solve that.
>>
>> This isn't used in any hot-path, so I'm not worried for now.
>>
>> If it comes up in the future it can be investigated then.
>>
>> Would you like to add a todo: ? or not worth it?
> 
> It's always good to keep todos in mind :-) Please see below.
> 
>>>>> +};
>>>>> +
>>>>> +} /* namespace details */
>>>>> +
>>>>> +details::StringSplitter split(const std::string &str, const std::string &delim);
>>>>> +
>>>>>  } /* namespace utils */
>>>>>  
>>>>>  } /* namespace libcamera */
>>>>> diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp
>>>>> index 4beffdab5eb6..fe027c0b009c 100644
>>>>> --- a/src/libcamera/utils.cpp
>>>>> +++ b/src/libcamera/utils.cpp
>>>>> @@ -199,6 +199,68 @@ size_t strlcpy(char *dst, const char *src, size_t size)
>>>>>  	return strlen(src);
>>>>>  }
>>>>>  
>>>>> +details::StringSplitter::StringSplitter(const std::string &str, const std::string &delim)
>>>>> +	: str_(str), delim_(delim)
>>>>> +{
>>>>> +}
>>>>> +
>>>>> +details::StringSplitter::iterator::iterator(const details::StringSplitter *ss, std::string::size_type pos)
>>>>> +	: ss_(ss), pos_(pos)
>>>>> +{
>>>>> +	next_ = ss_->str_.find(ss_->delim_, pos_);
>>>>> +}
>>>>> +
>>>>> +details::StringSplitter::iterator &details::StringSplitter::iterator::operator++()
>>>>> +{
>>>>> +	pos_ = next_;
>>>>> +	if (pos_ != std::string::npos) {
>>>>> +		pos_ += ss_->delim_.length();
>>>>> +		next_ = ss_->str_.find(ss_->delim_, pos_);
>>>>> +	}
>>>>> +
>>>>> +	return *this;
>>>>> +}
>>>>> +
>>>>> +std::string details::StringSplitter::iterator::operator*() const
>>>>> +{
>>>>> +	std::string::size_type count;
>>>>> +	count = next_ != std::string::npos ? next_ - pos_ : next_;
>>>>> +	return ss_->str_.substr(pos_, count);
>>>>> +}
>>>>> +
>>>>> +bool details::StringSplitter::iterator::operator!=(const details::StringSplitter::iterator &other) const
>>>>> +{
>>>>> +	return pos_ != other.pos_;
>>>>> +}
>>>>> +
>>>>> +details::StringSplitter::iterator details::StringSplitter::begin() const
>>>>> +{
>>>>> +	return iterator(this, 0);
>>>>> +}
>>>>> +
>>>>> +details::StringSplitter::iterator details::StringSplitter::end() const
>>>>> +{
>>>>> +	return iterator(this, std::string::npos);
>>>>> +}
>>>>> +
>>>>> +/**
>>>>> + * \fn split(const std::string &str, const std::string &delim)
>>>>> + * \brief Split a string based on a delimiter
>>>>> + * \param[in] str The string to split
>>>>> + * \param[in] delim The delimiter string
>>>>> + *
>>>>> + * This function splits the string \a str into substrings based on the
>>>>> + * delimiter \a delim. It returns an object of unspecified type that can be
>>>>> + * used in a range-based for loop and yields the substrings in sequence.
>>>>> + *
>>>>> + * \return An object that can be used in a range-based for loop to iterate over
>>>>> + * the substrings
>>>>> + */
>>>>> +details::StringSplitter split(const std::string &str, const std::string &delim)
>>>>> +{
> 
> 	/** \todo Try to avoid copies of str and delim */
> 

I'll add this and push the series.

Thanks.

--
KB

>>>>> +	return details::StringSplitter(str, delim);
>>>>> +}
>>>>> +
>>>>>  } /* namespace utils */
>>>>>  
>>>>>  } /* namespace libcamera */
>

Patch
diff mbox series 

diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in
index 1c46b04b3f7e..beeaf6d3cf48 100644
--- a/Documentation/Doxyfile.in
+++ b/Documentation/Doxyfile.in
@@ -880,6 +880,7 @@  EXCLUDE_SYMBOLS        = libcamera::BoundMethodArgs \
                          libcamera::BoundMethodStatic \
                          libcamera::SignalBase \
                          libcamera::*::Private \
+                         libcamera::*::details::* \
                          std::*
 
 # The EXAMPLE_PATH tag can be used to specify one or more files or directories
diff --git a/src/libcamera/include/utils.h b/src/libcamera/include/utils.h
index e467eb21c518..080ea6614de0 100644
--- a/src/libcamera/include/utils.h
+++ b/src/libcamera/include/utils.h
@@ -108,6 +108,40 @@  inline _hex hex<uint64_t>(uint64_t value, unsigned int width)
 
 size_t strlcpy(char *dst, const char *src, size_t size);
 
+namespace details {
+
+class StringSplitter
+{
+public:
+	StringSplitter(const std::string &str, const std::string &delim);
+
+	class iterator
+	{
+	public:
+		iterator(const StringSplitter *ss, std::string::size_type pos);
+
+		iterator &operator++();
+		std::string operator*() const;
+		bool operator!=(const iterator &other) const;
+
+	private:
+		const StringSplitter *ss_;
+		std::string::size_type pos_;
+		std::string::size_type next_;
+	};
+
+	iterator begin() const;
+	iterator end() const;
+
+private:
+	std::string str_;
+	std::string delim_;
+};
+
+} /* namespace details */
+
+details::StringSplitter split(const std::string &str, const std::string &delim);
+
 } /* namespace utils */
 
 } /* namespace libcamera */
diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp
index 4beffdab5eb6..fe027c0b009c 100644
--- a/src/libcamera/utils.cpp
+++ b/src/libcamera/utils.cpp
@@ -199,6 +199,68 @@  size_t strlcpy(char *dst, const char *src, size_t size)
 	return strlen(src);
 }
 
+details::StringSplitter::StringSplitter(const std::string &str, const std::string &delim)
+	: str_(str), delim_(delim)
+{
+}
+
+details::StringSplitter::iterator::iterator(const details::StringSplitter *ss, std::string::size_type pos)
+	: ss_(ss), pos_(pos)
+{
+	next_ = ss_->str_.find(ss_->delim_, pos_);
+}
+
+details::StringSplitter::iterator &details::StringSplitter::iterator::operator++()
+{
+	pos_ = next_;
+	if (pos_ != std::string::npos) {
+		pos_ += ss_->delim_.length();
+		next_ = ss_->str_.find(ss_->delim_, pos_);
+	}
+
+	return *this;
+}
+
+std::string details::StringSplitter::iterator::operator*() const
+{
+	std::string::size_type count;
+	count = next_ != std::string::npos ? next_ - pos_ : next_;
+	return ss_->str_.substr(pos_, count);
+}
+
+bool details::StringSplitter::iterator::operator!=(const details::StringSplitter::iterator &other) const
+{
+	return pos_ != other.pos_;
+}
+
+details::StringSplitter::iterator details::StringSplitter::begin() const
+{
+	return iterator(this, 0);
+}
+
+details::StringSplitter::iterator details::StringSplitter::end() const
+{
+	return iterator(this, std::string::npos);
+}
+
+/**
+ * \fn split(const std::string &str, const std::string &delim)
+ * \brief Split a string based on a delimiter
+ * \param[in] str The string to split
+ * \param[in] delim The delimiter string
+ *
+ * This function splits the string \a str into substrings based on the
+ * delimiter \a delim. It returns an object of unspecified type that can be
+ * used in a range-based for loop and yields the substrings in sequence.
+ *
+ * \return An object that can be used in a range-based for loop to iterate over
+ * the substrings
+ */
+details::StringSplitter split(const std::string &str, const std::string &delim)
+{
+	return details::StringSplitter(str, delim);
+}
+
 } /* namespace utils */
 
 } /* namespace libcamera */