From ef5faac6ffdb6bce688d2468d653ef456d8f85f1 Mon Sep 17 00:00:00 2001 From: flipflip Date: Thu, 29 Oct 2026 13:07:24 +0100 Subject: [PATCH 1/2] Prevent NmeaCollectGsaGsv memory usage growing indefinitely --- fpsdk_common/include/fpsdk_common/parser/nmea.hpp | 15 +++++++++++++++ fpsdk_common/src/parser/nmea.cpp | 15 +++++++++++++-- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/fpsdk_common/include/fpsdk_common/parser/nmea.hpp b/fpsdk_common/include/fpsdk_common/parser/nmea.hpp index 79d9b8a..6fd729b 100644 --- a/fpsdk_common/include/fpsdk_common/parser/nmea.hpp +++ b/fpsdk_common/include/fpsdk_common/parser/nmea.hpp @@ -859,6 +859,11 @@ inline NmeaPayloadPtr NmeaDecodeMessage(const std::vector& msg) /** * @brief Collector for NMEA-Gx-GSA and NMEA-Gx-GPA + * + * Note that the user is responsible to call Complete() on every end of epoch. This collector by itself has no means to + * determine start or end of epoch. If Complete() is not called, the Add...() methods refuse to collect more data after + * a certain threshold (see MAX_COLLECT below) and fail. This is to prevent memory growing forever if Complete() is not + * called appropriately. */ struct NmeaCollectGsaGsv { @@ -929,6 +934,16 @@ struct NmeaCollectGsaGsv private: std::vector gsa_sats_; //!< Satellites used + + /** + * @brief Maximum number of entries to collect for any of sats_, sigs_ or gsa_sats_ + * + * This is to prevent the memory usage growing over time if the user keeps adding data and never calls Complete(). + * The value is large enough to cover the maximum number of satellits and signals a receiver will see. This should + * never be reached when the collector is used properly (by calling Complete() on end of epoch). + * + */ + static constexpr std::size_t MAX_COLLECT = 1000; }; /* ****************************************************************************************************************** */ diff --git a/fpsdk_common/src/parser/nmea.cpp b/fpsdk_common/src/parser/nmea.cpp index 10a79a4..fa63b47 100644 --- a/fpsdk_common/src/parser/nmea.cpp +++ b/fpsdk_common/src/parser/nmea.cpp @@ -25,6 +25,7 @@ /* EXTERNAL */ /* PACKAGE */ +#include "fpsdk_common/logging.hpp" #include "fpsdk_common/math.hpp" #include "fpsdk_common/parser/nmea.hpp" #include "fpsdk_common/parser/types.hpp" @@ -500,9 +501,9 @@ static bool StrToInt(const std::string& str, int32_t& value) // Debug prints, for development #if 0 -# define NMEA_TRACE(fmt, ...) fprintf(stderr, fmt "\n", ##__VA_ARGS__) +# define NMEA_TRACE(...) TRACE(__VA_ARGS__) #else -# define NMEA_TRACE(fmt, ...) /* nothing */ +# define NMEA_TRACE(...) /* nothing */ #endif // Split NMEA sentence (message) into its meta data and the payload fields @@ -1409,6 +1410,11 @@ bool NmeaCollectGsaGsv::AddGsaAndGsv(const std::vector& gsas, co bool NmeaCollectGsaGsv::AddGsa(const NmeaGsaPayload& gsa) { + if (gsa_sats_.size() > MAX_COLLECT) { + WARNING_THR(10000, "NmeaCollectGsaGsv: too many NMEA-Gx-GSA in epoch"); + return false; + } + // Collect all "satellites used" from GSA message. They have no numbering, so we have to rely on the user giving us // all messages. for (auto& gsa_sat : gsa.sats) { @@ -1428,6 +1434,11 @@ bool NmeaCollectGsaGsv::AddGsa(const NmeaGsaPayload& gsa) bool NmeaCollectGsaGsv::AddGsv(const NmeaGsvPayload& gsv) { + if ((sats_.size() > MAX_COLLECT) || (sigs_.size() > MAX_COLLECT)) { + WARNING_THR(10000, "NmeaCollectGsaGsv: too many NMEA-Gx-GSV in epoch"); + return false; + } + // @todo: Add checks on expected sequence of messages maybe? We could use the msg_num and num_msgs fields and check // that a set of messages for each GNSS was seen. Check for duplicate SVs, ... From a3232d014c7c413c538238eef98a4617434850ee Mon Sep 17 00:00:00 2001 From: flipflip Date: Thu, 29 Oct 2026 13:08:13 +0100 Subject: [PATCH 2/2] codenazi --- fpsdk_common/include/fpsdk_common/parser/nmea.hpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/fpsdk_common/include/fpsdk_common/parser/nmea.hpp b/fpsdk_common/include/fpsdk_common/parser/nmea.hpp index 6fd729b..96dcd36 100644 --- a/fpsdk_common/include/fpsdk_common/parser/nmea.hpp +++ b/fpsdk_common/include/fpsdk_common/parser/nmea.hpp @@ -936,12 +936,11 @@ struct NmeaCollectGsaGsv std::vector gsa_sats_; //!< Satellites used /** - * @brief Maximum number of entries to collect for any of sats_, sigs_ or gsa_sats_ - * - * This is to prevent the memory usage growing over time if the user keeps adding data and never calls Complete(). - * The value is large enough to cover the maximum number of satellits and signals a receiver will see. This should - * never be reached when the collector is used properly (by calling Complete() on end of epoch). - * + * @brief Maximum number of entries to collect for any of sats_, sigs_ or gsa_sats_ + * + * This is to prevent the memory usage growing over time if the user keeps adding data and never calls Complete(). + * The value is large enough to cover the maximum number of satellits and signals a receiver will see. This should + * never be reached when the collector is used properly (by calling Complete() on end of epoch). */ static constexpr std::size_t MAX_COLLECT = 1000; };