From patchwork Wed Jul 27 02:38:03 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 16813 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 6D7F7BE173 for ; Wed, 27 Jul 2022 02:38:21 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id D9C1363313; Wed, 27 Jul 2022 04:38:19 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1658889499; bh=tA5v39SUMYbMaj+WnM1dGYwfSN0EUnXF/y7pX/wddrU=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=v8AogBPWRz49bYI1wLc4w79GQlm6JAolfHXjfl9yr2uSsu/7/XXoFfkbkT5+oIo/+ 87wtCm2IZ4tTXc5K8CPuEL+34Nsbkp/d8qGuSPVn8N4EXmibRJ2a5Pp1CdLTd/guDH 2Bw0E/5oD28P7Y9mMj8so42HNeNUSOKjtr/keP2sPR2d9QcLnlOeNqydmMLoyBrzPD l1AlA31+QFcHwmE+oxqA25RF6K7Gjj6H4ASKVp7AnyMUpHP6rxz2I5ZzZtwuJqwi7q XwS4+tY1DKgiU4i3jy9S489qvfO2GCsCor5rDNCsMwomKvjtNx9hYHX/BX+P8LDrfJ GzBZcU49yMA9w== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 30C656330E for ; Wed, 27 Jul 2022 04:38:19 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="ZElNNomz"; dkim-atps=neutral Received: from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id BE20956D; Wed, 27 Jul 2022 04:38:18 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1658889499; bh=tA5v39SUMYbMaj+WnM1dGYwfSN0EUnXF/y7pX/wddrU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=ZElNNomzFUFGL7BtWa4MskABgSi17AQMqdkSS7RhdKJtCUMd0XbF9EYGkBGmyPn9h WvMlpZ2iDJfdJgDAh+qEezr5zJehTp1fAXqdDtimVvJIQNJNUrLRdA0mJZ2BvkvGmC 4wFGYviRL+u7ARhvN2+nHVDEagj+zD3lpLnQ99sE= To: libcamera-devel@lists.libcamera.org Date: Wed, 27 Jul 2022 05:38:03 +0300 Message-Id: <20220727023816.30008-2-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20220727023816.30008-1-laurent.pinchart@ideasonboard.com> References: <20220727023816.30008-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v7 01/14] libcamera: yaml_parser: Replace ok flag to get() with std::optional X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Laurent Pinchart via libcamera-devel From: Laurent Pinchart Reply-To: Laurent Pinchart Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" 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 Reviewed-by: Naushir Patuck --- 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 #include +#include #include #include @@ -165,7 +166,13 @@ public: #else template #endif - T get(const T &defaultValue, bool *ok = nullptr) const; + std::optional get() const; + + template + T get(const T &defaultValue) const + { + return get().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 YamlObject::get( - * const T &defaultValue, bool *ok) const + * \fn template YamlObject::get() 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 YamlObject::get(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 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 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::min() || value > std::numeric_limits::max()) - return defaultValue; + return {}; - setOk(ok, true); return value; } template<> -uint16_t YamlObject::get(const uint16_t &defaultValue, bool *ok) const +std::optional 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::min() || value > std::numeric_limits::max()) - return defaultValue; + return {}; - setOk(ok, true); return value; } template<> -int32_t YamlObject::get(const int32_t &defaultValue, bool *ok) const +std::optional 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::min() || value > std::numeric_limits::max()) - return defaultValue; + return {}; - setOk(ok, true); return value; } template<> -uint32_t YamlObject::get(const uint32_t &defaultValue, bool *ok) const +std::optional 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::min() || value > std::numeric_limits::max()) - return defaultValue; + return {}; - setOk(ok, true); return value; } template<> -double YamlObject::get(const double &defaultValue, bool *ok) const +std::optional 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 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 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(0, &valid); - if (!valid) - return defaultValue; + auto width = list_[0]->get(); + if (!width) + return {}; - uint32_t height = list_[1]->get(0, &valid); - if (!valid) - return defaultValue; + auto height = list_[1]->get(); + 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("", &ok) != "libcamera" || !ok) { + if (strObj.get().value_or("") != "libcamera") { cerr << "String object parse as wrong content" << std::endl; return TestFail; } - if (strObj.get(-1, &ok) != -1 || ok) { + if (strObj.get()) { cerr << "String object parse as integer" << std::endl; return TestFail; } - if (strObj.get(1, &ok) != 1 || ok) { + if (strObj.get()) { cerr << "String object parse as unsigned integer" << std::endl; return TestFail; } - if (strObj.get(1.0, &ok) != 1.0 || ok) { + if (strObj.get()) { cerr << "String object parse as double" << std::endl; return TestFail; } - if (strObj.get(Size(0, 0), &ok) != Size(0, 0) || ok) { + if (strObj.get()) { cerr << "String object parse as Size" << std::endl; return TestFail; } @@ -199,27 +198,27 @@ protected: return TestFail; } - if (int32Obj.get(-100, &ok) != -100 || !ok) { + if (int32Obj.get().value_or(0) != -100) { cerr << "Integer object parse as wrong value" << std::endl; return TestFail; } - if (int32Obj.get("", &ok) != "-100" || !ok) { + if (int32Obj.get().value_or("") != "-100") { cerr << "Integer object fail to parse as string" << std::endl; return TestFail; } - if (int32Obj.get(1.0, &ok) != -100.0 || !ok) { + if (int32Obj.get().value_or(0.0) != -100.0) { cerr << "Integer object fail to parse as double" << std::endl; return TestFail; } - if (int32Obj.get(1, &ok) != 1 || ok) { + if (int32Obj.get()) { cerr << "Negative integer object parse as unsigned integer" << std::endl; return TestFail; } - if (int32Obj.get(Size(0, 0), &ok) != Size(0, 0) || ok) { + if (int32Obj.get()) { cerr << "Integer object parse as Size" << std::endl; return TestFail; } @@ -237,27 +236,27 @@ protected: return TestFail; } - if (uint32Obj.get(-1, &ok) != 100 || !ok) { + if (uint32Obj.get().value_or(0) != 100) { cerr << "Unsigned integer object fail to parse as integer" << std::endl; return TestFail; } - if (uint32Obj.get("", &ok) != "100" || !ok) { + if (uint32Obj.get().value_or("") != "100") { cerr << "Unsigned integer object fail to parse as string" << std::endl; return TestFail; } - if (uint32Obj.get(1.0, &ok) != 100.0 || !ok) { + if (uint32Obj.get().value_or(0.0) != 100.0) { cerr << "Unsigned integer object fail to parse as double" << std::endl; return TestFail; } - if (uint32Obj.get(100, &ok) != 100 || !ok) { + if (uint32Obj.get().value_or(0) != 100) { cerr << "Unsigned integer object parsed as wrong value" << std::endl; return TestFail; } - if (uint32Obj.get(Size(0, 0), &ok) != Size(0, 0) || ok) { + if (uint32Obj.get()) { cerr << "Unsigned integer object parsed as Size" << std::endl; return TestFail; } @@ -275,27 +274,27 @@ protected: return TestFail; } - if (doubleObj.get("", &ok) != "3.14159" || !ok) { + if (doubleObj.get().value_or("") != "3.14159") { cerr << "Double object fail to parse as string" << std::endl; return TestFail; } - if (doubleObj.get(1.0, &ok) != 3.14159 || !ok) { + if (doubleObj.get().value_or(0.0) != 3.14159) { cerr << "Double object parse as wrong value" << std::endl; return TestFail; } - if (doubleObj.get(-1, &ok) != -1 || ok) { + if (doubleObj.get()) { cerr << "Double object parse as integer" << std::endl; return TestFail; } - if (doubleObj.get(1, &ok) != 1 || ok) { + if (doubleObj.get()) { cerr << "Double object parse as unsigned integer" << std::endl; return TestFail; } - if (doubleObj.get(Size(0, 0), &ok) != Size(0, 0) || ok) { + if (doubleObj.get()) { cerr << "Double object parse as Size" << std::endl; return TestFail; } @@ -313,27 +312,27 @@ protected: return TestFail; } - if (sizeObj.get("", &ok) != "" || ok) { + if (sizeObj.get()) { cerr << "Size object parse as string" << std::endl; return TestFail; } - if (sizeObj.get(1.0, &ok) != 1.0 || ok) { + if (sizeObj.get()) { cerr << "Size object parse as double" << std::endl; return TestFail; } - if (sizeObj.get(-1, &ok) != -1 || ok) { + if (sizeObj.get()) { cerr << "Size object parse as integer" << std::endl; return TestFail; } - if (sizeObj.get(1, &ok) != 1 || ok) { + if (sizeObj.get()) { cerr << "Size object parse as unsigned integer" << std::endl; return TestFail; } - if (sizeObj.get(Size(0, 0), &ok) != Size(1920, 1080) || !ok) { + if (sizeObj.get().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("", &ok) != "" || ok) { + if (listObj.get()) { cerr << "List object parse as string" << std::endl; return TestFail; } - if (listObj.get(1.0, &ok) != 1.0 || ok) { + if (listObj.get()) { cerr << "List object parse as double" << std::endl; return TestFail; } - if (listObj.get(-1, &ok) != -1 || ok) { + if (listObj.get()) { cerr << "List object parse as integer" << std::endl; return TestFail; } - if (listObj.get(1, &ok) != 1 || ok) { + if (listObj.get()) { cerr << "List object parse as unsigne integer" << std::endl; return TestFail; } - if (listObj.get(Size(0, 0), &ok) != Size(0, 0) || ok) { + if (listObj.get()) { cerr << "String list object parse as Size" << std::endl; return TestFail; } @@ -424,27 +423,27 @@ protected: return TestFail; } - if (dictObj.get("", &ok) != "" || ok) { + if (dictObj.get()) { cerr << "Dictionary object parse as string" << std::endl; return TestFail; } - if (dictObj.get(1.0, &ok) != 1.0 || ok) { + if (dictObj.get()) { cerr << "Dictionary object parse as double" << std::endl; return TestFail; } - if (dictObj.get(-1, &ok) != -1 || ok) { + if (dictObj.get()) { cerr << "Dictionary object parse as integer" << std::endl; return TestFail; } - if (dictObj.get(1, &ok) != 1 || ok) { + if (dictObj.get()) { cerr << "Dictionary object parse as unsigned integer" << std::endl; return TestFail; } - if (dictObj.get(Size(0, 0), &ok) != Size(0, 0) || ok) { + if (dictObj.get()) { cerr << "Dictionary object parse as Size" << std::endl; return TestFail; }