[v11,03/12] config: Add configuration retrieval helpers

23631 diff mbox series

Let's add some helpers to make accessing simple configuration values
simpler.

GlobalConfiguration::option ensures that no value is returned rather
than a value of YamlObject::empty.

Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
---
 .../libcamera/internal/global_configuration.h |  5 ++
 src/libcamera/global_configuration.cpp        | 52 +++++++++++++++++++
 2 files changed, 57 insertions(+)

Hi

2025. 06. 24. 10:36 keltezéssel, Milan Zamazal írta:
> Let's add some helpers to make accessing simple configuration values
> simpler.
> 
> GlobalConfiguration::option ensures that no value is returned rather
> than a value of YamlObject::empty.
> 
> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
> ---
>   .../libcamera/internal/global_configuration.h |  5 ++
>   src/libcamera/global_configuration.cpp        | 52 +++++++++++++++++++
>   2 files changed, 57 insertions(+)
> 
> diff --git a/include/libcamera/internal/global_configuration.h b/include/libcamera/internal/global_configuration.h
> index 89984c84d..357119628 100644
> --- a/include/libcamera/internal/global_configuration.h
> +++ b/include/libcamera/internal/global_configuration.h
> @@ -8,6 +8,8 @@
>   #pragma once
>   
>   #include <filesystem>
> +#include <optional>
> +#include <string>
>   
>   #include "libcamera/internal/yaml_parser.h"
>   
> @@ -22,6 +24,9 @@ public:
>   
>   	unsigned int version() const;
>   	Configuration configuration() const;
> +	std::optional<std::string> option(const std::string &confPath) const;
> +	std::optional<std::string> envOption(const char *const envVariable,
> +					     const std::string &confPath) const;
>   
>   private:
>   	bool loadFile(const std::filesystem::path &fileName);
> diff --git a/src/libcamera/global_configuration.cpp b/src/libcamera/global_configuration.cpp
> index 72b4a5246..068ddd5a5 100644
> --- a/src/libcamera/global_configuration.cpp
> +++ b/src/libcamera/global_configuration.cpp
> @@ -8,6 +8,8 @@
>   #include "libcamera/internal/global_configuration.h"
>   
>   #include <filesystem>
> +#include <memory>
> +#include <string>
>   #include <string_view>
>   #include <sys/types.h>
>   
> @@ -39,6 +41,11 @@ LOG_DEFINE_CATEGORY(Configuration)
>    * is not found then in system-wide configuration directories. If multiple
>    * configuration files exist then only the first one found is used and no
>    * configuration merging is performed.
> + *
> + * The configuration can be accessed using the provided helpers. Namely
> + * GlobalConfiguration::option() or GlobalConfiguration::envOption() to access
> + * individual options or GlobalConfiguration::configuration() to access the
> + * whole configuration.
>    */
>   
>   bool GlobalConfiguration::loadFile(const std::filesystem::path &fileName)
> @@ -108,6 +115,51 @@ GlobalConfiguration::GlobalConfiguration()
>    * the underlying type.
>    */
>   
> +/**
> + * \brief Return value of the configuration option identified by \a confPath
> + * \param[in] confPath Sequence of the YAML section names (excluding
> + * `configuration') leading to the requested option separated by slashes
> + * \return A value if an item corresponding to \a confPath exists in the
> + * configuration file, no value otherwise
> + */
> +std::optional<std::string> GlobalConfiguration::option(const std::string &confPath) const

I think we should avoid in-band signalling, so I would prefer removing the separators from the
string. And instead making the function take an `std::initializer_list<std::string_view>` (and
`Span<const std::string_view>` if an init list not sufficient, but I think it should be).
I believe it is inevitable that someone will forget to consider if the input is "safe",
especially if not all path components are fixed.


> +{
> +	const YamlObject *c = &configuration();
> +	for (auto part : utils::split(confPath, "/")) {
> +		c = &(*c)[part];
> +		if (!*c)
> +			return {};
> +	}
> +	return c->get<std::string>();
> +}
> +
> +/**
> + * \brief Return value of the configuration option from a file or environment
> + * \param[in] envVariable Environment variable to get the value from
> + * \param[in] confPath The same as in GlobalConfiguration::option
> + *
> + * This helper looks first at the given environment variable and if it is
> + * defined then it returns its value (even if it is empty). Otherwise it looks
> + * for \a confPath the same way as in GlobalConfiguration::option. Only string
> + * values are supported.
> + *
> + * \note Support for using environment variables to configure libcamera behavior
> + * is provided here mostly for backward compatibility reasons. Introducing new
> + * configuration environment variables is discouraged.
> + *
> + * \return A value retrieved from the given environment option or configuration
> + * file or no value if not found
> + */
> +std::optional<std::string> GlobalConfiguration::envOption(
> +	const char *const envVariable,

Could you please drop the top-level `const`?


Regards,
Barnabás Pőcze


> +	const std::string &confPath) const
> +{
> +	const char *envValue = utils::secure_getenv(envVariable);
> +	if (envValue)
> +		return std::optional{ std::string{ envValue } };
> +	return option(confPath);
> +}
> +
>   /**
>    * \brief Return configuration version
>    *

Hi Barnabás,

thank you for review.

Barnabás Pőcze <barnabas.pocze@ideasonboard.com> writes:

> Hi
>
> 2025. 06. 24. 10:36 keltezéssel, Milan Zamazal írta:
>> Let's add some helpers to make accessing simple configuration values
>> simpler.
>> GlobalConfiguration::option ensures that no value is returned rather
>> than a value of YamlObject::empty.
>> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
>> ---
>>   .../libcamera/internal/global_configuration.h |  5 ++
>>   src/libcamera/global_configuration.cpp        | 52 +++++++++++++++++++
>>   2 files changed, 57 insertions(+)
>> diff --git a/include/libcamera/internal/global_configuration.h
>> b/include/libcamera/internal/global_configuration.h
>> index 89984c84d..357119628 100644
>> --- a/include/libcamera/internal/global_configuration.h
>> +++ b/include/libcamera/internal/global_configuration.h
>> @@ -8,6 +8,8 @@
>>   #pragma once
>>     #include <filesystem>
>> +#include <optional>
>> +#include <string>
>>     #include "libcamera/internal/yaml_parser.h"
>>   @@ -22,6 +24,9 @@ public:
>>     	unsigned int version() const;
>>   	Configuration configuration() const;
>> +	std::optional<std::string> option(const std::string &confPath) const;
>> +	std::optional<std::string> envOption(const char *const envVariable,
>> +					     const std::string &confPath) const;
>>     private:
>>   	bool loadFile(const std::filesystem::path &fileName);
>> diff --git a/src/libcamera/global_configuration.cpp b/src/libcamera/global_configuration.cpp
>> index 72b4a5246..068ddd5a5 100644
>> --- a/src/libcamera/global_configuration.cpp
>> +++ b/src/libcamera/global_configuration.cpp
>> @@ -8,6 +8,8 @@
>>   #include "libcamera/internal/global_configuration.h"
>>     #include <filesystem>
>> +#include <memory>
>> +#include <string>
>>   #include <string_view>
>>   #include <sys/types.h>
>>   @@ -39,6 +41,11 @@ LOG_DEFINE_CATEGORY(Configuration)
>>    * is not found then in system-wide configuration directories. If multiple
>>    * configuration files exist then only the first one found is used and no
>>    * configuration merging is performed.
>> + *
>> + * The configuration can be accessed using the provided helpers. Namely
>> + * GlobalConfiguration::option() or GlobalConfiguration::envOption() to access
>> + * individual options or GlobalConfiguration::configuration() to access the
>> + * whole configuration.
>>    */
>>     bool GlobalConfiguration::loadFile(const std::filesystem::path &fileName)
>> @@ -108,6 +115,51 @@ GlobalConfiguration::GlobalConfiguration()
>>    * the underlying type.
>>    */
>>   +/**
>> + * \brief Return value of the configuration option identified by \a confPath
>> + * \param[in] confPath Sequence of the YAML section names (excluding
>> + * `configuration') leading to the requested option separated by slashes
>> + * \return A value if an item corresponding to \a confPath exists in the
>> + * configuration file, no value otherwise
>> + */
>> +std::optional<std::string> GlobalConfiguration::option(const std::string &confPath) const
>
> I think we should avoid in-band signalling, so I would prefer removing the separators from the
> string. And instead making the function take an `std::initializer_list<std::string_view>` (and
> `Span<const std::string_view>` if an init list not sufficient, but I think it should be).
> I believe it is inevitable that someone will forget to consider if the input is "safe",
> especially if not all path components are fixed.

A good idea, will do.

>> +{
>> +	const YamlObject *c = &configuration();
>> +	for (auto part : utils::split(confPath, "/")) {
>> +		c = &(*c)[part];
>> +		if (!*c)
>> +			return {};
>> +	}
>> +	return c->get<std::string>();
>> +}
>> +
>> +/**
>> + * \brief Return value of the configuration option from a file or environment
>> + * \param[in] envVariable Environment variable to get the value from
>> + * \param[in] confPath The same as in GlobalConfiguration::option
>> + *
>> + * This helper looks first at the given environment variable and if it is
>> + * defined then it returns its value (even if it is empty). Otherwise it looks
>> + * for \a confPath the same way as in GlobalConfiguration::option. Only string
>> + * values are supported.
>> + *
>> + * \note Support for using environment variables to configure libcamera behavior
>> + * is provided here mostly for backward compatibility reasons. Introducing new
>> + * configuration environment variables is discouraged.
>> + *
>> + * \return A value retrieved from the given environment option or configuration
>> + * file or no value if not found
>> + */
>> +std::optional<std::string> GlobalConfiguration::envOption(
>> +	const char *const envVariable,
>
> Could you please drop the top-level `const`?

OK.

> Regards,
> Barnabás Pőcze
>
>
>> +	const std::string &confPath) const
>> +{
>> +	const char *envValue = utils::secure_getenv(envVariable);
>> +	if (envValue)
>> +		return std::optional{ std::string{ envValue } };
>> +	return option(confPath);
>> +}
>> +
>>   /**
>>    * \brief Return configuration version
>>    *