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

[Jacopo Mondi]

Hi Laurent

On Fri, Dec 02, 2022 at 10:31:26PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> 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 ?

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