[libcamera-devel] [PATCH 2/2] libcamera: utils: Provide strchrnul() wrapper

[Laurent Pinchart]

Hi Jacopo,

On Mon, Dec 05, 2022 at 09:48:51AM +0100, Jacopo Mondi wrote:
> On Fri, Dec 02, 2022 at 10:31:26PM +0200, Laurent Pinchart wrote:
> > On Fri, Dec 02, 2022 at 05:41:45PM +0100, Jacopo Mondi via libcamera-devel wrote:
> > > The strchrnul function is a GNU-specific extension to string.h and it's
> > > not availalable on all C libraries (in example, Android's Bionic).
> >
> > It has been added to bionic in API level 24.
> >
> > Which API level are you using ? Would it be feasible to require level 24
> > ? It seems to have been introduced in Android 7.0, first released in
> > 2016. Do we need to support anything older than that ?
> 
> Lineage OS 18.1 which is based on Android 11, API level 21.
> 
> No idea what is the Android release cycle but 11 seems to still be in
> use ?

Android 11 has shipped with API level 30. There's something odd there
that we need to understand better.

> > Same question for lockf(), which also got introduce in API level 24.
> >
> > > Provide an implementation of the function based around the generally
> > > available strchr() function and use it in the Logger class.
> > >
> > > Open code the same wrapper in the application's option parser, where we
> > > cannot use the internal utils namespace.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > > ---
> > >  include/libcamera/base/utils.h |  1 +
> > >  src/apps/common/options.cpp    |  5 ++++-
> > >  src/libcamera/base/log.cpp     |  2 +-
> > >  src/libcamera/base/utils.cpp   | 19 +++++++++++++++++++
> > >  4 files changed, 25 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/include/libcamera/base/utils.h b/include/libcamera/base/utils.h
> > > index eb7bcdf4c173..b7064ec6253b 100644
> > > --- a/include/libcamera/base/utils.h
> > > +++ b/include/libcamera/base/utils.h
> > > @@ -36,6 +36,7 @@ namespace libcamera {
> > >  namespace utils {
> > >
> > >  const char *basename(const char *path);
> > > +const char *strchrnul(const char *s, int c);
> > >
> > >  char *secure_getenv(const char *name);
> > >  std::string dirname(const std::string &path);
> > > diff --git a/src/apps/common/options.cpp b/src/apps/common/options.cpp
> > > index 4f7e869144c8..1bbbec7fa25c 100644
> > > --- a/src/apps/common/options.cpp
> > > +++ b/src/apps/common/options.cpp
> > > @@ -363,7 +363,10 @@ KeyValueParser::Options KeyValueParser::parse(const char *arguments)
> > >  	Options options;
> > >
> > >  	for (const char *pair = arguments; *arguments != '\0'; pair = arguments) {
> > > -		const char *comma = strchrnul(arguments, ',');
> > > +		const char *comma = strchr(arguments, ',');
> > > +		if (!comma)
> > > +			comma = &arguments[strlen(arguments)];
> > > +
> > >  		size_t len = comma - pair;
> > >
> > >  		/* Skip over the comma. */
> > > diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp
> > > index 55fbd7b03438..cc30681fd67f 100644
> > > --- a/src/libcamera/base/log.cpp
> > > +++ b/src/libcamera/base/log.cpp
> > > @@ -629,7 +629,7 @@ void Logger::parseLogLevels()
> > >  		return;
> > >
> > >  	for (const char *pair = debug; *debug != '\0'; pair = debug) {
> > > -		const char *comma = strchrnul(debug, ',');
> > > +		const char *comma = utils::strchrnul(debug, ',');
> > >  		size_t len = comma - pair;
> > >
> > >  		/* Skip over the comma. */
> > > diff --git a/src/libcamera/base/utils.cpp b/src/libcamera/base/utils.cpp
> > > index 6a307940448e..f829a8106b76 100644
> > > --- a/src/libcamera/base/utils.cpp
> > > +++ b/src/libcamera/base/utils.cpp
> > > @@ -39,6 +39,25 @@ const char *basename(const char *path)
> > >  	return base ? base + 1 : path;
> > >  }
> > >
> > > +/**
> > > + * \brief Implement strchnul wrapper
> > > + * \param[in] s The string to seach on
> > > + * \param[in] c The character to search
> > > + *
> > > + * The strchrnul function is a GNU-specific extension to string.h and it's not
> > > + * available on all C libraries (in example, Android's Bionic). This
> > > + * implementation realizes strchrnul() on strchr() which is instead more
> > > + * generally available.
> > > + *
> > > + * \return A pointer to the first occurrence of \a c in \a s, or a pointer to
> > > + * the null byte at the end of \a s if \a c is not found
> > > + */
> > > +const char *strchrnul(const char *s, int c)
> > > +{
> > > +	const char *p = strchr(s, c);
> > > +	return p ? : s + strlen(s);
> > > +}
> > > +
> > >  /**
> > >   * \brief Get an environment variable
> > >   * \param[in] name The name of the variable to return

-- 
Regards,

Laurent Pinchart