[libcamera-devel,v7,01/14] libcamera: yaml_parser: Replace ok flag to get() with std::optional
diff mbox series

Message ID 20220727023816.30008-2-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • Replace boost JSON parser with libyaml in Raspberry Pi IPA
Related show

Commit Message

Laurent Pinchart July 27, 2022, 2:38 a.m. UTC 
The YamlObject::get() function takes a default value and an optional
bool ok flag to handle parsing errors. This ad-hoc mechanism complicates
error handling in callers.

A better API is possible by dropping the default value and ok flag and
returning an std::optional. Not only does it simplify the calls, it also
lets callers handle errors through the standard std::optional class
instead of the current ad-hoc mechanism.

Provide a convenience get() wrapper around std::optional::value_or() to
further simplify callers that don't need any specific error handling.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/internal/yaml_parser.h |   9 +-
 src/libcamera/yaml_parser.cpp            | 138 +++++++++--------------
 test/yaml-parser.cpp                     |  71 ++++++------
 3 files changed, 96 insertions(+), 122 deletions(-)

Comments

Naushir Patuck July 27, 2022, 9:34 a.m. UTC | #1 
Hi Laurent,

Thank you for your patch.

On Wed, 27 Jul 2022 at 03:38, Laurent Pinchart <
laurent.pinchart@ideasonboard.com> wrote:

> The YamlObject::get() function takes a default value and an optional
> bool ok flag to handle parsing errors. This ad-hoc mechanism complicates
> error handling in callers.
>
> A better API is possible by dropping the default value and ok flag and
> returning an std::optional. Not only does it simplify the calls, it also
> lets callers handle errors through the standard std::optional class
> instead of the current ad-hoc mechanism.
>
> Provide a convenience get() wrapper around std::optional::value_or() to
> further simplify callers that don't need any specific error handling.
>

s/convenience/convenient/ ?


> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>

This is much cleaner for error handling!

Reviewed-by: Naushir Patuck <naush@raspberrypi.com>


> ---
>  include/libcamera/internal/yaml_parser.h |   9 +-
>  src/libcamera/yaml_parser.cpp            | 138 +++++++++--------------
>  test/yaml-parser.cpp                     |  71 ++++++------
>  3 files changed, 96 insertions(+), 122 deletions(-)
>
> diff --git a/include/libcamera/internal/yaml_parser.h
> b/include/libcamera/internal/yaml_parser.h
> index 064cf44381d7..61f2223223a7 100644
> --- a/include/libcamera/internal/yaml_parser.h
> +++ b/include/libcamera/internal/yaml_parser.h
> @@ -9,6 +9,7 @@
>
>  #include <iterator>
>  #include <map>
> +#include <optional>
>  #include <string>
>  #include <vector>
>
> @@ -165,7 +166,13 @@ public:
>  #else
>         template<typename T>
>  #endif
> -       T get(const T &defaultValue, bool *ok = nullptr) const;
> +       std::optional<T> get() const;
> +
> +       template<typename T>
> +       T get(const T &defaultValue) const
> +       {
> +               return get<T>().value_or(defaultValue);
> +       }
>
>         DictAdapter asDict() const { return DictAdapter{ dictionary_ }; }
>         ListAdapter asList() const { return ListAdapter{ list_ }; }
> diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp
> index 5c45e44e49c3..4299f5abd38a 100644
> --- a/src/libcamera/yaml_parser.cpp
> +++ b/src/libcamera/yaml_parser.cpp
> @@ -31,12 +31,6 @@ namespace {
>  /* Empty static YamlObject as a safe result for invalid operations */
>  static const YamlObject empty;
>
> -void setOk(bool *ok, bool result)
> -{
> -       if (ok)
> -               *ok = result;
> -}
> -
>  } /* namespace */
>
>  /**
> @@ -100,54 +94,52 @@ std::size_t YamlObject::size() const
>  }
>
>  /**
> - * \fn template<typename T> YamlObject::get<T>(
> - *     const T &defaultValue, bool *ok) const
> + * \fn template<typename T> YamlObject::get<T>() const
> + * \brief Parse the YamlObject as a \a T value
> + *
> + * This function parses the value of the YamlObject as a \a T object, and
> + * returns the value. If parsing fails (usually because the YamlObject
> doesn't
> + * store a \a T value), std::nullopt is returned.
> + *
> + * \return The YamlObject value, or std::nullopt if parsing failed
> + */
> +
> +/**
> + * \fn template<typename T> YamlObject::get<T>(const T &defaultValue)
> const
>   * \brief Parse the YamlObject as a \a T value
>   * \param[in] defaultValue The default value when failing to parse
> - * \param[out] ok The result of whether the parse succeeded
>   *
>   * This function parses the value of the YamlObject as a \a T object, and
>   * returns the value. If parsing fails (usually because the YamlObject
> doesn't
> - * store a \a T value), the \a defaultValue is returned, and \a ok is set
> to
> - * false. Otherwise, the YamlObject value is returned, and \a ok is set
> to true.
> + * store a \a T value), the \a defaultValue is returned.
>   *
> - * The \a ok pointer is optional and can be a nullptr if the caller
> doesn't
> - * need to know if parsing succeeded.
> - *
> - * \return Value as a bool type
> + * \return The YamlObject value, or \a defaultValue if parsing failed
>   */
>
>  #ifndef __DOXYGEN__
>
>  template<>
> -bool YamlObject::get(const bool &defaultValue, bool *ok) const
> +std::optional<bool> YamlObject::get() const
>  {
> -       setOk(ok, false);
> -
>         if (type_ != Type::Value)
> -               return defaultValue;
> +               return {};
>
> -       if (value_ == "true") {
> -               setOk(ok, true);
> +       if (value_ == "true")
>                 return true;
> -       } else if (value_ == "false") {
> -               setOk(ok, true);
> +       else if (value_ == "false")
>                 return false;
> -       }
>
> -       return defaultValue;
> +       return {};
>  }
>
>  template<>
> -int16_t YamlObject::get(const int16_t &defaultValue, bool *ok) const
> +std::optional<int16_t> YamlObject::get() const
>  {
> -       setOk(ok, false);
> -
>         if (type_ != Type::Value)
> -               return defaultValue;
> +               return {};
>
>         if (value_ == "")
> -               return defaultValue;
> +               return {};
>
>         char *end;
>
> @@ -157,22 +149,19 @@ int16_t YamlObject::get(const int16_t &defaultValue,
> bool *ok) const
>         if ('\0' != *end || errno == ERANGE ||
>             value < std::numeric_limits<int16_t>::min() ||
>             value > std::numeric_limits<int16_t>::max())
> -               return defaultValue;
> +               return {};
>
> -       setOk(ok, true);
>         return value;
>  }
>
>  template<>
> -uint16_t YamlObject::get(const uint16_t &defaultValue, bool *ok) const
> +std::optional<uint16_t> YamlObject::get() const
>  {
> -       setOk(ok, false);
> -
>         if (type_ != Type::Value)
> -               return defaultValue;
> +               return {};
>
>         if (value_ == "")
> -               return defaultValue;
> +               return {};
>
>         /*
>          * libyaml parses all scalar values as strings. When a string has
> @@ -183,7 +172,7 @@ uint16_t YamlObject::get(const uint16_t &defaultValue,
> bool *ok) const
>          */
>         std::size_t found = value_.find_first_not_of(" \t");
>         if (found != std::string::npos && value_[found] == '-')
> -               return defaultValue;
> +               return {};
>
>         char *end;
>
> @@ -193,22 +182,19 @@ uint16_t YamlObject::get(const uint16_t
> &defaultValue, bool *ok) const
>         if ('\0' != *end || errno == ERANGE ||
>             value < std::numeric_limits<uint16_t>::min() ||
>             value > std::numeric_limits<uint16_t>::max())
> -               return defaultValue;
> +               return {};
>
> -       setOk(ok, true);
>         return value;
>  }
>
>  template<>
> -int32_t YamlObject::get(const int32_t &defaultValue, bool *ok) const
> +std::optional<int32_t> YamlObject::get() const
>  {
> -       setOk(ok, false);
> -
>         if (type_ != Type::Value)
> -               return defaultValue;
> +               return {};
>
>         if (value_ == "")
> -               return defaultValue;
> +               return {};
>
>         char *end;
>
> @@ -218,22 +204,19 @@ int32_t YamlObject::get(const int32_t &defaultValue,
> bool *ok) const
>         if ('\0' != *end || errno == ERANGE ||
>             value < std::numeric_limits<int32_t>::min() ||
>             value > std::numeric_limits<int32_t>::max())
> -               return defaultValue;
> +               return {};
>
> -       setOk(ok, true);
>         return value;
>  }
>
>  template<>
> -uint32_t YamlObject::get(const uint32_t &defaultValue, bool *ok) const
> +std::optional<uint32_t> YamlObject::get() const
>  {
> -       setOk(ok, false);
> -
>         if (type_ != Type::Value)
> -               return defaultValue;
> +               return {};
>
>         if (value_ == "")
> -               return defaultValue;
> +               return {};
>
>         /*
>          * libyaml parses all scalar values as strings. When a string has
> @@ -244,7 +227,7 @@ uint32_t YamlObject::get(const uint32_t &defaultValue,
> bool *ok) const
>          */
>         std::size_t found = value_.find_first_not_of(" \t");
>         if (found != std::string::npos && value_[found] == '-')
> -               return defaultValue;
> +               return {};
>
>         char *end;
>
> @@ -254,22 +237,19 @@ uint32_t YamlObject::get(const uint32_t
> &defaultValue, bool *ok) const
>         if ('\0' != *end || errno == ERANGE ||
>             value < std::numeric_limits<uint32_t>::min() ||
>             value > std::numeric_limits<uint32_t>::max())
> -               return defaultValue;
> +               return {};
>
> -       setOk(ok, true);
>         return value;
>  }
>
>  template<>
> -double YamlObject::get(const double &defaultValue, bool *ok) const
> +std::optional<double> YamlObject::get() const
>  {
> -       setOk(ok, false);
> -
>         if (type_ != Type::Value)
> -               return defaultValue;
> +               return {};
>
>         if (value_ == "")
> -               return defaultValue;
> +               return {};
>
>         char *end;
>
> @@ -277,50 +257,38 @@ double YamlObject::get(const double &defaultValue,
> bool *ok) const
>         double value = std::strtod(value_.c_str(), &end);
>
>         if ('\0' != *end || errno == ERANGE)
> -               return defaultValue;
> +               return {};
>
> -       setOk(ok, true);
>         return value;
>  }
>
>  template<>
> -std::string YamlObject::get(const std::string &defaultValue, bool *ok)
> const
> +std::optional<std::string> YamlObject::get() const
>  {
> -       setOk(ok, false);
> -
>         if (type_ != Type::Value)
> -               return defaultValue;
> +               return {};
>
> -       setOk(ok, true);
>         return value_;
>  }
>
>  template<>
> -Size YamlObject::get(const Size &defaultValue, bool *ok) const
> +std::optional<Size> YamlObject::get() const
>  {
> -       setOk(ok, false);
> -
>         if (type_ != Type::List)
> -               return defaultValue;
> +               return {};
>
>         if (list_.size() != 2)
> -               return defaultValue;
> +               return {};
>
> -       /*
> -        * Add a local variable to validate each dimension in case
> -        * that ok == nullptr.
> -        */
> -       bool valid;
> -       uint32_t width = list_[0]->get<uint32_t>(0, &valid);
> -       if (!valid)
> -               return defaultValue;
> +       auto width = list_[0]->get<uint32_t>();
> +       if (!width)
> +               return {};
>
> -       uint32_t height = list_[1]->get<uint32_t>(0, &valid);
> -       if (!valid)
> -               return defaultValue;
> +       auto height = list_[1]->get<uint32_t>();
> +       if (!height)
> +               return {};
>
> -       setOk(ok, true);
> -       return Size(width, height);
> +       return Size(*width, *height);
>  }
>
>  #endif /* __DOXYGEN__ */
> diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp
> index 38f848232fa6..ebb654f2ef9c 100644
> --- a/test/yaml-parser.cpp
> +++ b/test/yaml-parser.cpp
> @@ -148,7 +148,6 @@ protected:
>                 }
>
>                 /* Test string object */
> -               bool ok;
>                 auto &strObj = (*root)["string"];
>
>                 if (strObj.isDictionary()) {
> @@ -161,27 +160,27 @@ protected:
>                         return TestFail;
>                 }
>
> -               if (strObj.get<string>("", &ok) != "libcamera" || !ok) {
> +               if (strObj.get<string>().value_or("") != "libcamera") {
>                         cerr << "String object parse as wrong content" <<
> std::endl;
>                         return TestFail;
>                 }
>
> -               if (strObj.get<int32_t>(-1, &ok) != -1 || ok) {
> +               if (strObj.get<int32_t>()) {
>                         cerr << "String object parse as integer" <<
> std::endl;
>                         return TestFail;
>                 }
>
> -               if (strObj.get<uint32_t>(1, &ok) != 1 || ok) {
> +               if (strObj.get<uint32_t>()) {
>                         cerr << "String object parse as unsigned integer"
> << std::endl;
>                         return TestFail;
>                 }
>
> -               if (strObj.get<double>(1.0, &ok) != 1.0 || ok) {
> +               if (strObj.get<double>()) {
>                         cerr << "String object parse as double" <<
> std::endl;
>                         return TestFail;
>                 }
>
> -               if (strObj.get<Size>(Size(0, 0), &ok) != Size(0, 0) || ok)
> {
> +               if (strObj.get<Size>()) {
>                         cerr << "String object parse as Size" << std::endl;
>                         return TestFail;
>                 }
> @@ -199,27 +198,27 @@ protected:
>                         return TestFail;
>                 }
>
> -               if (int32Obj.get<int32_t>(-100, &ok) != -100 || !ok) {
> +               if (int32Obj.get<int32_t>().value_or(0) != -100) {
>                         cerr << "Integer object parse as wrong value" <<
> std::endl;
>                         return TestFail;
>                 }
>
> -               if (int32Obj.get<string>("", &ok) != "-100" || !ok) {
> +               if (int32Obj.get<string>().value_or("") != "-100") {
>                         cerr << "Integer object fail to parse as string"
> << std::endl;
>                         return TestFail;
>                 }
>
> -               if (int32Obj.get<double>(1.0, &ok) != -100.0 || !ok) {
> +               if (int32Obj.get<double>().value_or(0.0) != -100.0) {
>                         cerr << "Integer object fail to parse as double"
> << std::endl;
>                         return TestFail;
>                 }
>
> -               if (int32Obj.get<uint32_t>(1, &ok) != 1 || ok) {
> +               if (int32Obj.get<uint32_t>()) {
>                         cerr << "Negative integer object parse as unsigned
> integer" << std::endl;
>                         return TestFail;
>                 }
>
> -               if (int32Obj.get<Size>(Size(0, 0), &ok) != Size(0, 0) ||
> ok) {
> +               if (int32Obj.get<Size>()) {
>                         cerr << "Integer object parse as Size" <<
> std::endl;
>                         return TestFail;
>                 }
> @@ -237,27 +236,27 @@ protected:
>                         return TestFail;
>                 }
>
> -               if (uint32Obj.get<int32_t>(-1, &ok) != 100 || !ok) {
> +               if (uint32Obj.get<int32_t>().value_or(0) != 100) {
>                         cerr << "Unsigned integer object fail to parse as
> integer" << std::endl;
>                         return TestFail;
>                 }
>
> -               if (uint32Obj.get<string>("", &ok) != "100" || !ok) {
> +               if (uint32Obj.get<string>().value_or("") != "100") {
>                         cerr << "Unsigned integer object fail to parse as
> string" << std::endl;
>                         return TestFail;
>                 }
>
> -               if (uint32Obj.get<double>(1.0, &ok) != 100.0 || !ok) {
> +               if (uint32Obj.get<double>().value_or(0.0) != 100.0) {
>                         cerr << "Unsigned integer object fail to parse as
> double" << std::endl;
>                         return TestFail;
>                 }
>
> -               if (uint32Obj.get<uint32_t>(100, &ok) != 100 || !ok) {
> +               if (uint32Obj.get<uint32_t>().value_or(0) != 100) {
>                         cerr << "Unsigned integer object parsed as wrong
> value" << std::endl;
>                         return TestFail;
>                 }
>
> -               if (uint32Obj.get<Size>(Size(0, 0), &ok) != Size(0, 0) ||
> ok) {
> +               if (uint32Obj.get<Size>()) {
>                         cerr << "Unsigned integer object parsed as Size"
> << std::endl;
>                         return TestFail;
>                 }
> @@ -275,27 +274,27 @@ protected:
>                         return TestFail;
>                 }
>
> -               if (doubleObj.get<string>("", &ok) != "3.14159" || !ok) {
> +               if (doubleObj.get<string>().value_or("") != "3.14159") {
>                         cerr << "Double object fail to parse as string" <<
> std::endl;
>                         return TestFail;
>                 }
>
> -               if (doubleObj.get<double>(1.0, &ok) != 3.14159 || !ok) {
> +               if (doubleObj.get<double>().value_or(0.0) != 3.14159) {
>                         cerr << "Double object parse as wrong value" <<
> std::endl;
>                         return TestFail;
>                 }
>
> -               if (doubleObj.get<int32_t>(-1, &ok) != -1 || ok) {
> +               if (doubleObj.get<int32_t>()) {
>                         cerr << "Double object parse as integer" <<
> std::endl;
>                         return TestFail;
>                 }
>
> -               if (doubleObj.get<uint32_t>(1, &ok) != 1 || ok) {
> +               if (doubleObj.get<uint32_t>()) {
>                         cerr << "Double object parse as unsigned integer"
> << std::endl;
>                         return TestFail;
>                 }
>
> -               if (doubleObj.get<Size>(Size(0, 0), &ok) != Size(0, 0) ||
> ok) {
> +               if (doubleObj.get<Size>()) {
>                         cerr << "Double object parse as Size" << std::endl;
>                         return TestFail;
>                 }
> @@ -313,27 +312,27 @@ protected:
>                         return TestFail;
>                 }
>
> -               if (sizeObj.get<string>("", &ok) != "" || ok) {
> +               if (sizeObj.get<string>()) {
>                         cerr << "Size object parse as string" << std::endl;
>                         return TestFail;
>                 }
>
> -               if (sizeObj.get<double>(1.0, &ok) != 1.0 || ok) {
> +               if (sizeObj.get<double>()) {
>                         cerr << "Size object parse as double" << std::endl;
>                         return TestFail;
>                 }
>
> -               if (sizeObj.get<int32_t>(-1, &ok) != -1 || ok) {
> +               if (sizeObj.get<int32_t>()) {
>                         cerr << "Size object parse as integer" <<
> std::endl;
>                         return TestFail;
>                 }
>
> -               if (sizeObj.get<uint32_t>(1, &ok) != 1 || ok) {
> +               if (sizeObj.get<uint32_t>()) {
>                         cerr << "Size object parse as unsigned integer" <<
> std::endl;
>                         return TestFail;
>                 }
>
> -               if (sizeObj.get<Size>(Size(0, 0), &ok) != Size(1920, 1080)
> || !ok) {
> +               if (sizeObj.get<Size>().value_or(Size(0, 0)) != Size(1920,
> 1080)) {
>                         cerr << "Size object parse as wrong value" <<
> std::endl;
>                         return TestFail;
>                 }
> @@ -351,27 +350,27 @@ protected:
>                         return TestFail;
>                 }
>
> -               if (listObj.get<string>("", &ok) != "" || ok) {
> +               if (listObj.get<string>()) {
>                         cerr << "List object parse as string" << std::endl;
>                         return TestFail;
>                 }
>
> -               if (listObj.get<double>(1.0, &ok) != 1.0 || ok) {
> +               if (listObj.get<double>()) {
>                         cerr << "List object parse as double" << std::endl;
>                         return TestFail;
>                 }
>
> -               if (listObj.get<int32_t>(-1, &ok) != -1 || ok) {
> +               if (listObj.get<int32_t>()) {
>                         cerr << "List object parse as integer" <<
> std::endl;
>                         return TestFail;
>                 }
>
> -               if (listObj.get<uint32_t>(1, &ok) != 1 || ok) {
> +               if (listObj.get<uint32_t>()) {
>                         cerr << "List object parse as unsigne integer" <<
> std::endl;
>                         return TestFail;
>                 }
>
> -               if (listObj.get<Size>(Size(0, 0), &ok) != Size(0, 0) ||
> ok) {
> +               if (listObj.get<Size>()) {
>                         cerr << "String list object parse as Size" <<
> std::endl;
>                         return TestFail;
>                 }
> @@ -424,27 +423,27 @@ protected:
>                         return TestFail;
>                 }
>
> -               if (dictObj.get<string>("", &ok) != "" || ok) {
> +               if (dictObj.get<string>()) {
>                         cerr << "Dictionary object parse as string" <<
> std::endl;
>                         return TestFail;
>                 }
>
> -               if (dictObj.get<double>(1.0, &ok) != 1.0 || ok) {
> +               if (dictObj.get<double>()) {
>                         cerr << "Dictionary object parse as double" <<
> std::endl;
>                         return TestFail;
>                 }
>
> -               if (dictObj.get<int32_t>(-1, &ok) != -1 || ok) {
> +               if (dictObj.get<int32_t>()) {
>                         cerr << "Dictionary object parse as integer" <<
> std::endl;
>                         return TestFail;
>                 }
>
> -               if (dictObj.get<uint32_t>(1, &ok) != 1 || ok) {
> +               if (dictObj.get<uint32_t>()) {
>                         cerr << "Dictionary object parse as unsigned
> integer" << std::endl;
>                         return TestFail;
>                 }
>
> -               if (dictObj.get<Size>(Size(0, 0), &ok) != Size(0, 0) ||
> ok) {
> +               if (dictObj.get<Size>()) {
>                         cerr << "Dictionary object parse as Size" <<
> std::endl;
>                         return TestFail;
>                 }
> --
> Regards,
>
> Laurent Pinchart
>
>

Patch
diff mbox series 

diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h
index 064cf44381d7..61f2223223a7 100644
--- a/include/libcamera/internal/yaml_parser.h
+++ b/include/libcamera/internal/yaml_parser.h
@@ -9,6 +9,7 @@ 
 
 #include <iterator>
 #include <map>
+#include <optional>
 #include <string>
 #include <vector>
 
@@ -165,7 +166,13 @@  public:
 #else
 	template<typename T>
 #endif
-	T get(const T &defaultValue, bool *ok = nullptr) const;
+	std::optional<T> get() const;
+
+	template<typename T>
+	T get(const T &defaultValue) const
+	{
+		return get<T>().value_or(defaultValue);
+	}
 
 	DictAdapter asDict() const { return DictAdapter{ dictionary_ }; }
 	ListAdapter asList() const { return ListAdapter{ list_ }; }
diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp
index 5c45e44e49c3..4299f5abd38a 100644
--- a/src/libcamera/yaml_parser.cpp
+++ b/src/libcamera/yaml_parser.cpp
@@ -31,12 +31,6 @@  namespace {
 /* Empty static YamlObject as a safe result for invalid operations */
 static const YamlObject empty;
 
-void setOk(bool *ok, bool result)
-{
-	if (ok)
-		*ok = result;
-}
-
 } /* namespace */
 
 /**
@@ -100,54 +94,52 @@  std::size_t YamlObject::size() const
 }
 
 /**
- * \fn template<typename T> YamlObject::get<T>(
- *	const T &defaultValue, bool *ok) const
+ * \fn template<typename T> YamlObject::get<T>() const
+ * \brief Parse the YamlObject as a \a T value
+ *
+ * This function parses the value of the YamlObject as a \a T object, and
+ * returns the value. If parsing fails (usually because the YamlObject doesn't
+ * store a \a T value), std::nullopt is returned.
+ *
+ * \return The YamlObject value, or std::nullopt if parsing failed
+ */
+
+/**
+ * \fn template<typename T> YamlObject::get<T>(const T &defaultValue) const
  * \brief Parse the YamlObject as a \a T value
  * \param[in] defaultValue The default value when failing to parse
- * \param[out] ok The result of whether the parse succeeded
  *
  * This function parses the value of the YamlObject as a \a T object, and
  * returns the value. If parsing fails (usually because the YamlObject doesn't
- * store a \a T value), the \a defaultValue is returned, and \a ok is set to
- * false. Otherwise, the YamlObject value is returned, and \a ok is set to true.
+ * store a \a T value), the \a defaultValue is returned.
  *
- * The \a ok pointer is optional and can be a nullptr if the caller doesn't
- * need to know if parsing succeeded.
- *
- * \return Value as a bool type
+ * \return The YamlObject value, or \a defaultValue if parsing failed
  */
 
 #ifndef __DOXYGEN__
 
 template<>
-bool YamlObject::get(const bool &defaultValue, bool *ok) const
+std::optional<bool> YamlObject::get() const
 {
-	setOk(ok, false);
-
 	if (type_ != Type::Value)
-		return defaultValue;
+		return {};
 
-	if (value_ == "true") {
-		setOk(ok, true);
+	if (value_ == "true")
 		return true;
-	} else if (value_ == "false") {
-		setOk(ok, true);
+	else if (value_ == "false")
 		return false;
-	}
 
-	return defaultValue;
+	return {};
 }
 
 template<>
-int16_t YamlObject::get(const int16_t &defaultValue, bool *ok) const
+std::optional<int16_t> YamlObject::get() const
 {
-	setOk(ok, false);
-
 	if (type_ != Type::Value)
-		return defaultValue;
+		return {};
 
 	if (value_ == "")
-		return defaultValue;
+		return {};
 
 	char *end;
 
@@ -157,22 +149,19 @@  int16_t YamlObject::get(const int16_t &defaultValue, bool *ok) const
 	if ('\0' != *end || errno == ERANGE ||
 	    value < std::numeric_limits<int16_t>::min() ||
 	    value > std::numeric_limits<int16_t>::max())
-		return defaultValue;
+		return {};
 
-	setOk(ok, true);
 	return value;
 }
 
 template<>
-uint16_t YamlObject::get(const uint16_t &defaultValue, bool *ok) const
+std::optional<uint16_t> YamlObject::get() const
 {
-	setOk(ok, false);
-
 	if (type_ != Type::Value)
-		return defaultValue;
+		return {};
 
 	if (value_ == "")
-		return defaultValue;
+		return {};
 
 	/*
 	 * libyaml parses all scalar values as strings. When a string has
@@ -183,7 +172,7 @@  uint16_t YamlObject::get(const uint16_t &defaultValue, bool *ok) const
 	 */
 	std::size_t found = value_.find_first_not_of(" \t");
 	if (found != std::string::npos && value_[found] == '-')
-		return defaultValue;
+		return {};
 
 	char *end;
 
@@ -193,22 +182,19 @@  uint16_t YamlObject::get(const uint16_t &defaultValue, bool *ok) const
 	if ('\0' != *end || errno == ERANGE ||
 	    value < std::numeric_limits<uint16_t>::min() ||
 	    value > std::numeric_limits<uint16_t>::max())
-		return defaultValue;
+		return {};
 
-	setOk(ok, true);
 	return value;
 }
 
 template<>
-int32_t YamlObject::get(const int32_t &defaultValue, bool *ok) const
+std::optional<int32_t> YamlObject::get() const
 {
-	setOk(ok, false);
-
 	if (type_ != Type::Value)
-		return defaultValue;
+		return {};
 
 	if (value_ == "")
-		return defaultValue;
+		return {};
 
 	char *end;
 
@@ -218,22 +204,19 @@  int32_t YamlObject::get(const int32_t &defaultValue, bool *ok) const
 	if ('\0' != *end || errno == ERANGE ||
 	    value < std::numeric_limits<int32_t>::min() ||
 	    value > std::numeric_limits<int32_t>::max())
-		return defaultValue;
+		return {};
 
-	setOk(ok, true);
 	return value;
 }
 
 template<>
-uint32_t YamlObject::get(const uint32_t &defaultValue, bool *ok) const
+std::optional<uint32_t> YamlObject::get() const
 {
-	setOk(ok, false);
-
 	if (type_ != Type::Value)
-		return defaultValue;
+		return {};
 
 	if (value_ == "")
-		return defaultValue;
+		return {};
 
 	/*
 	 * libyaml parses all scalar values as strings. When a string has
@@ -244,7 +227,7 @@  uint32_t YamlObject::get(const uint32_t &defaultValue, bool *ok) const
 	 */
 	std::size_t found = value_.find_first_not_of(" \t");
 	if (found != std::string::npos && value_[found] == '-')
-		return defaultValue;
+		return {};
 
 	char *end;
 
@@ -254,22 +237,19 @@  uint32_t YamlObject::get(const uint32_t &defaultValue, bool *ok) const
 	if ('\0' != *end || errno == ERANGE ||
 	    value < std::numeric_limits<uint32_t>::min() ||
 	    value > std::numeric_limits<uint32_t>::max())
-		return defaultValue;
+		return {};
 
-	setOk(ok, true);
 	return value;
 }
 
 template<>
-double YamlObject::get(const double &defaultValue, bool *ok) const
+std::optional<double> YamlObject::get() const
 {
-	setOk(ok, false);
-
 	if (type_ != Type::Value)
-		return defaultValue;
+		return {};
 
 	if (value_ == "")
-		return defaultValue;
+		return {};
 
 	char *end;
 
@@ -277,50 +257,38 @@  double YamlObject::get(const double &defaultValue, bool *ok) const
 	double value = std::strtod(value_.c_str(), &end);
 
 	if ('\0' != *end || errno == ERANGE)
-		return defaultValue;
+		return {};
 
-	setOk(ok, true);
 	return value;
 }
 
 template<>
-std::string YamlObject::get(const std::string &defaultValue, bool *ok) const
+std::optional<std::string> YamlObject::get() const
 {
-	setOk(ok, false);
-
 	if (type_ != Type::Value)
-		return defaultValue;
+		return {};
 
-	setOk(ok, true);
 	return value_;
 }
 
 template<>
-Size YamlObject::get(const Size &defaultValue, bool *ok) const
+std::optional<Size> YamlObject::get() const
 {
-	setOk(ok, false);
-
 	if (type_ != Type::List)
-		return defaultValue;
+		return {};
 
 	if (list_.size() != 2)
-		return defaultValue;
+		return {};
 
-	/*
-	 * Add a local variable to validate each dimension in case
-	 * that ok == nullptr.
-	 */
-	bool valid;
-	uint32_t width = list_[0]->get<uint32_t>(0, &valid);
-	if (!valid)
-		return defaultValue;
+	auto width = list_[0]->get<uint32_t>();
+	if (!width)
+		return {};
 
-	uint32_t height = list_[1]->get<uint32_t>(0, &valid);
-	if (!valid)
-		return defaultValue;
+	auto height = list_[1]->get<uint32_t>();
+	if (!height)
+		return {};
 
-	setOk(ok, true);
-	return Size(width, height);
+	return Size(*width, *height);
 }
 
 #endif /* __DOXYGEN__ */
diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp
index 38f848232fa6..ebb654f2ef9c 100644
--- a/test/yaml-parser.cpp
+++ b/test/yaml-parser.cpp
@@ -148,7 +148,6 @@  protected:
 		}
 
 		/* Test string object */
-		bool ok;
 		auto &strObj = (*root)["string"];
 
 		if (strObj.isDictionary()) {
@@ -161,27 +160,27 @@  protected:
 			return TestFail;
 		}
 
-		if (strObj.get<string>("", &ok) != "libcamera" || !ok) {
+		if (strObj.get<string>().value_or("") != "libcamera") {
 			cerr << "String object parse as wrong content" << std::endl;
 			return TestFail;
 		}
 
-		if (strObj.get<int32_t>(-1, &ok) != -1 || ok) {
+		if (strObj.get<int32_t>()) {
 			cerr << "String object parse as integer" << std::endl;
 			return TestFail;
 		}
 
-		if (strObj.get<uint32_t>(1, &ok) != 1 || ok) {
+		if (strObj.get<uint32_t>()) {
 			cerr << "String object parse as unsigned integer" << std::endl;
 			return TestFail;
 		}
 
-		if (strObj.get<double>(1.0, &ok) != 1.0 || ok) {
+		if (strObj.get<double>()) {
 			cerr << "String object parse as double" << std::endl;
 			return TestFail;
 		}
 
-		if (strObj.get<Size>(Size(0, 0), &ok) != Size(0, 0) || ok) {
+		if (strObj.get<Size>()) {
 			cerr << "String object parse as Size" << std::endl;
 			return TestFail;
 		}
@@ -199,27 +198,27 @@  protected:
 			return TestFail;
 		}
 
-		if (int32Obj.get<int32_t>(-100, &ok) != -100 || !ok) {
+		if (int32Obj.get<int32_t>().value_or(0) != -100) {
 			cerr << "Integer object parse as wrong value" << std::endl;
 			return TestFail;
 		}
 
-		if (int32Obj.get<string>("", &ok) != "-100" || !ok) {
+		if (int32Obj.get<string>().value_or("") != "-100") {
 			cerr << "Integer object fail to parse as string" << std::endl;
 			return TestFail;
 		}
 
-		if (int32Obj.get<double>(1.0, &ok) != -100.0 || !ok) {
+		if (int32Obj.get<double>().value_or(0.0) != -100.0) {
 			cerr << "Integer object fail to parse as double" << std::endl;
 			return TestFail;
 		}
 
-		if (int32Obj.get<uint32_t>(1, &ok) != 1 || ok) {
+		if (int32Obj.get<uint32_t>()) {
 			cerr << "Negative integer object parse as unsigned integer" << std::endl;
 			return TestFail;
 		}
 
-		if (int32Obj.get<Size>(Size(0, 0), &ok) != Size(0, 0) || ok) {
+		if (int32Obj.get<Size>()) {
 			cerr << "Integer object parse as Size" << std::endl;
 			return TestFail;
 		}
@@ -237,27 +236,27 @@  protected:
 			return TestFail;
 		}
 
-		if (uint32Obj.get<int32_t>(-1, &ok) != 100 || !ok) {
+		if (uint32Obj.get<int32_t>().value_or(0) != 100) {
 			cerr << "Unsigned integer object fail to parse as integer" << std::endl;
 			return TestFail;
 		}
 
-		if (uint32Obj.get<string>("", &ok) != "100" || !ok) {
+		if (uint32Obj.get<string>().value_or("") != "100") {
 			cerr << "Unsigned integer object fail to parse as string" << std::endl;
 			return TestFail;
 		}
 
-		if (uint32Obj.get<double>(1.0, &ok) != 100.0 || !ok) {
+		if (uint32Obj.get<double>().value_or(0.0) != 100.0) {
 			cerr << "Unsigned integer object fail to parse as double" << std::endl;
 			return TestFail;
 		}
 
-		if (uint32Obj.get<uint32_t>(100, &ok) != 100 || !ok) {
+		if (uint32Obj.get<uint32_t>().value_or(0) != 100) {
 			cerr << "Unsigned integer object parsed as wrong value" << std::endl;
 			return TestFail;
 		}
 
-		if (uint32Obj.get<Size>(Size(0, 0), &ok) != Size(0, 0) || ok) {
+		if (uint32Obj.get<Size>()) {
 			cerr << "Unsigned integer object parsed as Size" << std::endl;
 			return TestFail;
 		}
@@ -275,27 +274,27 @@  protected:
 			return TestFail;
 		}
 
-		if (doubleObj.get<string>("", &ok) != "3.14159" || !ok) {
+		if (doubleObj.get<string>().value_or("") != "3.14159") {
 			cerr << "Double object fail to parse as string" << std::endl;
 			return TestFail;
 		}
 
-		if (doubleObj.get<double>(1.0, &ok) != 3.14159 || !ok) {
+		if (doubleObj.get<double>().value_or(0.0) != 3.14159) {
 			cerr << "Double object parse as wrong value" << std::endl;
 			return TestFail;
 		}
 
-		if (doubleObj.get<int32_t>(-1, &ok) != -1 || ok) {
+		if (doubleObj.get<int32_t>()) {
 			cerr << "Double object parse as integer" << std::endl;
 			return TestFail;
 		}
 
-		if (doubleObj.get<uint32_t>(1, &ok) != 1 || ok) {
+		if (doubleObj.get<uint32_t>()) {
 			cerr << "Double object parse as unsigned integer" << std::endl;
 			return TestFail;
 		}
 
-		if (doubleObj.get<Size>(Size(0, 0), &ok) != Size(0, 0) || ok) {
+		if (doubleObj.get<Size>()) {
 			cerr << "Double object parse as Size" << std::endl;
 			return TestFail;
 		}
@@ -313,27 +312,27 @@  protected:
 			return TestFail;
 		}
 
-		if (sizeObj.get<string>("", &ok) != "" || ok) {
+		if (sizeObj.get<string>()) {
 			cerr << "Size object parse as string" << std::endl;
 			return TestFail;
 		}
 
-		if (sizeObj.get<double>(1.0, &ok) != 1.0 || ok) {
+		if (sizeObj.get<double>()) {
 			cerr << "Size object parse as double" << std::endl;
 			return TestFail;
 		}
 
-		if (sizeObj.get<int32_t>(-1, &ok) != -1 || ok) {
+		if (sizeObj.get<int32_t>()) {
 			cerr << "Size object parse as integer" << std::endl;
 			return TestFail;
 		}
 
-		if (sizeObj.get<uint32_t>(1, &ok) != 1 || ok) {
+		if (sizeObj.get<uint32_t>()) {
 			cerr << "Size object parse as unsigned integer" << std::endl;
 			return TestFail;
 		}
 
-		if (sizeObj.get<Size>(Size(0, 0), &ok) != Size(1920, 1080) || !ok) {
+		if (sizeObj.get<Size>().value_or(Size(0, 0)) != Size(1920, 1080)) {
 			cerr << "Size object parse as wrong value" << std::endl;
 			return TestFail;
 		}
@@ -351,27 +350,27 @@  protected:
 			return TestFail;
 		}
 
-		if (listObj.get<string>("", &ok) != "" || ok) {
+		if (listObj.get<string>()) {
 			cerr << "List object parse as string" << std::endl;
 			return TestFail;
 		}
 
-		if (listObj.get<double>(1.0, &ok) != 1.0 || ok) {
+		if (listObj.get<double>()) {
 			cerr << "List object parse as double" << std::endl;
 			return TestFail;
 		}
 
-		if (listObj.get<int32_t>(-1, &ok) != -1 || ok) {
+		if (listObj.get<int32_t>()) {
 			cerr << "List object parse as integer" << std::endl;
 			return TestFail;
 		}
 
-		if (listObj.get<uint32_t>(1, &ok) != 1 || ok) {
+		if (listObj.get<uint32_t>()) {
 			cerr << "List object parse as unsigne integer" << std::endl;
 			return TestFail;
 		}
 
-		if (listObj.get<Size>(Size(0, 0), &ok) != Size(0, 0) || ok) {
+		if (listObj.get<Size>()) {
 			cerr << "String list object parse as Size" << std::endl;
 			return TestFail;
 		}
@@ -424,27 +423,27 @@  protected:
 			return TestFail;
 		}
 
-		if (dictObj.get<string>("", &ok) != "" || ok) {
+		if (dictObj.get<string>()) {
 			cerr << "Dictionary object parse as string" << std::endl;
 			return TestFail;
 		}
 
-		if (dictObj.get<double>(1.0, &ok) != 1.0 || ok) {
+		if (dictObj.get<double>()) {
 			cerr << "Dictionary object parse as double" << std::endl;
 			return TestFail;
 		}
 
-		if (dictObj.get<int32_t>(-1, &ok) != -1 || ok) {
+		if (dictObj.get<int32_t>()) {
 			cerr << "Dictionary object parse as integer" << std::endl;
 			return TestFail;
 		}
 
-		if (dictObj.get<uint32_t>(1, &ok) != 1 || ok) {
+		if (dictObj.get<uint32_t>()) {
 			cerr << "Dictionary object parse as unsigned integer" << std::endl;
 			return TestFail;
 		}
 
-		if (dictObj.get<Size>(Size(0, 0), &ok) != Size(0, 0) || ok) {
+		if (dictObj.get<Size>()) {
 			cerr << "Dictionary object parse as Size" << std::endl;
 			return TestFail;
 		}