From 4319b4ad696caec78d957f36587b97a4caf7b8eb Mon Sep 17 00:00:00 2001 From: Andreas Suter Date: Sat, 6 Jun 2026 11:54:33 +0200 Subject: [PATCH] PStringUtils::ToInt: signal conversion errors via from_chars ToInt() previously wrapped strtol() with a nullptr endptr, so a failed conversion was indistinguishable from a legitimate 0 (matching the old TString::Atoi() behaviour). Switch the implementation to std::from_chars and add an optional `bool *ok` out-parameter that reports success: it is set to false on a non-numeric string or an out-of-range value, true otherwise. Leading whitespace is skipped and trailing characters are ignored, preserving the Atoi-like prefix semantics. The parameter defaults to nullptr, so existing call sites keep compiling unchanged. Convert the parse-validation call sites in PMsrHandler to the single -parse ToInt(token, &ok) form, replacing the IsInt() guard + separate ToInt() (which parsed every token twice). All downstream >0 / >=0 / range / enum checks are preserved. Left untouched the call sites where IsInt() acts as a structural discriminator rather than a numeric validator (write path, xy-data index-vs-label, fParamInUse usage scans) and the IsFloat-guarded ToInt offsets, where switching to ToInt(&ok) would change parsing semantics. All 85 integration tests pass. Co-Authored-By: Claude Opus 4.8 --- src/classes/PMsrHandler.cpp | 243 +++++++++++++++-------------------- src/classes/PStringUtils.cpp | 27 +++- src/include/PStringUtils.h | 10 +- 3 files changed, 135 insertions(+), 145 deletions(-) diff --git a/src/classes/PMsrHandler.cpp b/src/classes/PMsrHandler.cpp index 7db1597e3..0d6326265 100644 --- a/src/classes/PMsrHandler.cpp +++ b/src/classes/PMsrHandler.cpp @@ -2865,9 +2865,9 @@ Bool_t PMsrHandler::HandleFitParameterEntry(PMsrLines &lines) error = true; } else { // handle the first 4 parameter since they are always the same // parameter number - if (PStringUtils::IsInt(tokens[0])) - param.fNo = PStringUtils::ToInt(tokens[0]); - else + bool ok = false; + param.fNo = PStringUtils::ToInt(tokens[0], &ok); + if (!ok) error = true; // parameter name @@ -3107,19 +3107,16 @@ Bool_t PMsrHandler::HandleGlobalEntry(PMsrLines &lines) if (tokens.size() < 2) { error = true; } else { - if (PStringUtils::IsInt(tokens[1])) { - Int_t fittype = PStringUtils::ToInt(tokens[1]); - if ((fittype == MSR_FITTYPE_SINGLE_HISTO) || - (fittype == MSR_FITTYPE_SINGLE_HISTO_RRF) || - (fittype == MSR_FITTYPE_ASYM) || - (fittype == MSR_FITTYPE_ASYM_RRF) || - (fittype == MSR_FITTYPE_MU_MINUS) || - (fittype == MSR_FITTYPE_BNMR) || - (fittype == MSR_FITTYPE_NON_MUSR)) { - global.SetFitType(fittype); - } else { - error = true; - } + bool ok = false; + Int_t fittype = PStringUtils::ToInt(tokens[1], &ok); + if (ok && ((fittype == MSR_FITTYPE_SINGLE_HISTO) || + (fittype == MSR_FITTYPE_SINGLE_HISTO_RRF) || + (fittype == MSR_FITTYPE_ASYM) || + (fittype == MSR_FITTYPE_ASYM_RRF) || + (fittype == MSR_FITTYPE_MU_MINUS) || + (fittype == MSR_FITTYPE_BNMR) || + (fittype == MSR_FITTYPE_NON_MUSR))) { + global.SetFitType(fittype); } else { error = true; } @@ -3143,13 +3140,10 @@ Bool_t PMsrHandler::HandleGlobalEntry(PMsrLines &lines) if (tokens.size() < 2) { error = true; } else { - if (PStringUtils::IsInt(tokens[1])) { - ival = PStringUtils::ToInt(tokens[1]); - if (ival > 0) { - global.SetRRFPacking(ival); - } else { - error = true; - } + bool ok = false; + ival = PStringUtils::ToInt(tokens[1], &ok); + if (ok && ival > 0) { + global.SetRRFPacking(ival); } else { error = true; } @@ -3170,13 +3164,10 @@ Bool_t PMsrHandler::HandleGlobalEntry(PMsrLines &lines) error = true; } else { for (UInt_t i=1; i= 0) { - global.SetDataRange(ival, i-1); - } else { - error = true; - } + bool ok = false; + ival = PStringUtils::ToInt(tokens[i], &ok); + if (ok && ival >= 0) { + global.SetDataRange(ival, i-1); } else { error = true; } @@ -3261,13 +3252,10 @@ Bool_t PMsrHandler::HandleGlobalEntry(PMsrLines &lines) if (tokens.size() < 2) { error = true; } else { - if (PStringUtils::IsInt(tokens[1])) { - ival = PStringUtils::ToInt(tokens[1]); - if (ival >= 0) { - global.SetPacking(ival); - } else { - error = true; - } + bool ok = false; + ival = PStringUtils::ToInt(tokens[1], &ok); + if (ok && ival >= 0) { + global.SetPacking(ival); } else { error = true; } @@ -3432,19 +3420,16 @@ Bool_t PMsrHandler::HandleRunEntry(PMsrLines &lines) if (tokens.size() < 2) { error = true; } else { - if (PStringUtils::IsInt(tokens[1])) { - Int_t fittype = PStringUtils::ToInt(tokens[1]); - if ((fittype == MSR_FITTYPE_SINGLE_HISTO) || - (fittype == MSR_FITTYPE_SINGLE_HISTO_RRF) || - (fittype == MSR_FITTYPE_ASYM) || - (fittype == MSR_FITTYPE_ASYM_RRF) || - (fittype == MSR_FITTYPE_MU_MINUS) || - (fittype == MSR_FITTYPE_BNMR) || - (fittype == MSR_FITTYPE_NON_MUSR)) { - param.SetFitType(fittype); - } else { - error = true; - } + bool ok = false; + Int_t fittype = PStringUtils::ToInt(tokens[1], &ok); + if (ok && ((fittype == MSR_FITTYPE_SINGLE_HISTO) || + (fittype == MSR_FITTYPE_SINGLE_HISTO_RRF) || + (fittype == MSR_FITTYPE_ASYM) || + (fittype == MSR_FITTYPE_ASYM_RRF) || + (fittype == MSR_FITTYPE_MU_MINUS) || + (fittype == MSR_FITTYPE_BNMR) || + (fittype == MSR_FITTYPE_NON_MUSR))) { + param.SetFitType(fittype); } else { error = true; } @@ -3459,8 +3444,9 @@ Bool_t PMsrHandler::HandleRunEntry(PMsrLines &lines) if (tokens.size() < 2) { error = true; } else { - if (PStringUtils::IsInt(tokens[1])) { - ival = PStringUtils::ToInt(tokens[1]); + bool ok = false; + ival = PStringUtils::ToInt(tokens[1], &ok); + if (ok) { if (ival > 0) param.SetAlphaParamNo(ival); else @@ -3485,8 +3471,9 @@ Bool_t PMsrHandler::HandleRunEntry(PMsrLines &lines) if (tokens.size() < 2) { error = true; } else { - if (PStringUtils::IsInt(tokens[1])) { - ival = PStringUtils::ToInt(tokens[1]); + bool ok = false; + ival = PStringUtils::ToInt(tokens[1], &ok); + if (ok) { if (ival > 0) param.SetBetaParamNo(ival); else @@ -3511,8 +3498,10 @@ Bool_t PMsrHandler::HandleRunEntry(PMsrLines &lines) if (tokens.size() < 2) { error = true; } else { - if (PStringUtils::IsInt(tokens[1])) { - param.SetNormParamNo(PStringUtils::ToInt(tokens[1])); + bool ok = false; + ival = PStringUtils::ToInt(tokens[1], &ok); + if (ok) { + param.SetNormParamNo(ival); } else if (tokens[1].find("fun") != std::string::npos) { Int_t no; if (FilterNumber(tokens[1].c_str(), "fun", MSR_PARAM_FUN_OFFSET, no)) @@ -3533,15 +3522,12 @@ Bool_t PMsrHandler::HandleRunEntry(PMsrLines &lines) if (tokens.size() < 2) { error = true; } else { - if (PStringUtils::IsInt(tokens[1])) { - ival = PStringUtils::ToInt(tokens[1]); - if (ival > 0) - param.SetBkgFitParamNo(ival); - else - error = true; - } else { + bool ok = false; + ival = PStringUtils::ToInt(tokens[1], &ok); + if (ok && ival > 0) + param.SetBkgFitParamNo(ival); + else error = true; - } } } @@ -3553,15 +3539,12 @@ Bool_t PMsrHandler::HandleRunEntry(PMsrLines &lines) if (tokens.size() < 2) { error = true; } else { - if (PStringUtils::IsInt(tokens[1])) { - ival = PStringUtils::ToInt(tokens[1]); - if (ival > 0) - param.SetLifetimeParamNo(ival); - else - error = true; - } else { + bool ok = false; + ival = PStringUtils::ToInt(tokens[1], &ok); + if (ok && ival > 0) + param.SetLifetimeParamNo(ival); + else error = true; - } } } @@ -3579,15 +3562,12 @@ Bool_t PMsrHandler::HandleRunEntry(PMsrLines &lines) runLinePresent = false; // this is needed to make sure that a run line is present before and ADDRUN is following for (UInt_t i=1; i= 0) - param.SetMap(ival); - else - error = true; - } else { + bool ok = false; + ival = PStringUtils::ToInt(tokens[i], &ok); + if (ok && ival >= 0) + param.SetMap(ival); + else error = true; - } } // check map entries, i.e. if the map values are within parameter bounds if (!fFourierOnly) { @@ -3678,15 +3658,12 @@ Bool_t PMsrHandler::HandleRunEntry(PMsrLines &lines) error = true; } else { for (UInt_t i=1; i 0) - param.SetBkgRange(ival, i-1); - else - error = true; - } else { + bool ok = false; + ival = PStringUtils::ToInt(tokens[i], &ok); + if (ok && ival > 0) + param.SetBkgRange(ival, i-1); + else error = true; - } } } } @@ -3700,15 +3677,12 @@ Bool_t PMsrHandler::HandleRunEntry(PMsrLines &lines) error = true; } else { for (UInt_t i=1; i 0) - param.SetDataRange(ival, i-1); - else - error = true; - } else { + bool ok = false; + ival = PStringUtils::ToInt(tokens[i], &ok); + if (ok && ival > 0) + param.SetDataRange(ival, i-1); + else error = true; - } } } } @@ -3816,15 +3790,12 @@ Bool_t PMsrHandler::HandleRunEntry(PMsrLines &lines) if (tokens.size() != 2) { error = true; } else { - if (PStringUtils::IsInt(tokens[1])) { - ival = PStringUtils::ToInt(tokens[1]); - if (ival > 0) - param.SetPacking(ival); - else - error = true; - } else { + bool ok = false; + ival = PStringUtils::ToInt(tokens[1], &ok); + if (ok && ival > 0) + param.SetPacking(ival); + else error = true; - } } } @@ -4135,10 +4106,12 @@ Bool_t PMsrHandler::ParseFourierPhaseParVector(PMsrFourierStructure &fourier, co rmNoOf++; } sstr = sstr.substr(rmNoOf); // remove 'par' of 'parR' part. Rest should be an integer - if (PStringUtils::IsInt(sstr)) { + bool ok = false; + Int_t val = PStringUtils::ToInt(sstr, &ok); + if (ok) { if (rmNoOf == 4) // parR - fourier.fPhaseRef = PStringUtils::ToInt(sstr); - fourier.fPhaseParamNo.push_back(PStringUtils::ToInt(sstr)); + fourier.fPhaseRef = val; + fourier.fPhaseParamNo.push_back(val); } else { fLastErrorMsg.str(""); fLastErrorMsg.clear(); @@ -4222,9 +4195,9 @@ Bool_t PMsrHandler::ParseFourierPhaseParIterVector(PMsrFourierStructure &fourier Int_t x0, offset, noParam; // get X0 - if (PStringUtils::IsInt(tok[0])) { - x0 = PStringUtils::ToInt(tok[0]); - } else { + bool ok = false; + x0 = PStringUtils::ToInt(tok[0], &ok); + if (!ok) { fLastErrorMsg.str(""); fLastErrorMsg.clear(); fLastErrorMsg << ">> PMsrHandler::ParseFourierPhaseParIterVector: **ERROR** X0='" << tok[0] << "' is not an integer.\n"; @@ -4234,9 +4207,8 @@ Bool_t PMsrHandler::ParseFourierPhaseParIterVector(PMsrFourierStructure &fourier } // get offset - if (PStringUtils::IsInt(tok[1])) { - offset = PStringUtils::ToInt(tok[1]); - } else { + offset = PStringUtils::ToInt(tok[1], &ok); + if (!ok) { fLastErrorMsg.str(""); fLastErrorMsg.clear(); fLastErrorMsg << ">> PMsrHandler::ParseFourierPhaseParIterVector: **ERROR** offset='" << tok[1] << "' is not an integer.\n"; @@ -4246,9 +4218,8 @@ Bool_t PMsrHandler::ParseFourierPhaseParIterVector(PMsrFourierStructure &fourier } // get noParam - if (PStringUtils::IsInt(tok[2])) { - noParam = PStringUtils::ToInt(tok[2]); - } else { + noParam = PStringUtils::ToInt(tok[2], &ok); + if (!ok) { fLastErrorMsg.str(""); fLastErrorMsg.clear(); fLastErrorMsg << ">> PMsrHandler::ParseFourierPhaseParIterVector: **ERROR** #Param='" << tok[2] << "' is not an integer.\n"; @@ -4329,15 +4300,11 @@ Bool_t PMsrHandler::HandleFourierEntry(PMsrLines &lines) error = true; continue; } else { - if (PStringUtils::IsInt(tokens[1])) { - ival = PStringUtils::ToInt(tokens[1]); - if ((ival >= 0) && (ival <= 20)) { - fourier.fFourierPower = ival; - } else { // fourier power out of range - error = true; - continue; - } - } else { // fourier power not a number + bool ok = false; + ival = PStringUtils::ToInt(tokens[1], &ok); + if (ok && (ival >= 0) && (ival <= 20)) { + fourier.fFourierPower = ival; + } else { // fourier power not a number or out of range error = true; continue; } @@ -4616,9 +4583,9 @@ Bool_t PMsrHandler::HandlePlotEntry(PMsrLines &lines) if (tokens.size() < 2) { // plot type missing error = true; } else { - if (PStringUtils::IsInt(tokens[1])) - param.fPlotType = PStringUtils::ToInt(tokens[1]); - else + bool ok = false; + param.fPlotType = PStringUtils::ToInt(tokens[1], &ok); + if (!ok) error = true; } } else if (line.Contains("lifetimecorrection", TString::kIgnoreCase)) { @@ -4779,15 +4746,12 @@ Bool_t PMsrHandler::HandlePlotEntry(PMsrLines &lines) if (tokens.size() != 2) { error = true; } else { - if (PStringUtils::IsInt(tokens[1])) { - Int_t val = PStringUtils::ToInt(tokens[1]); - if (val > 0) - param.fViewPacking = val; - else - error = true; - } else { + bool ok = false; + Int_t val = PStringUtils::ToInt(tokens[1], &ok); + if (ok && val > 0) + param.fViewPacking = val; + else error = true; - } } } else if (iter1->fLine.Contains("rrf_freq", TString::kIgnoreCase)) { // expected entry: rrf_freq value unit @@ -4852,11 +4816,10 @@ Bool_t PMsrHandler::HandlePlotEntry(PMsrLines &lines) error = true; } else { // get rrf packing - if (PStringUtils::IsInt(tokens[1])) { - param.fRRFPacking = PStringUtils::ToInt(tokens[1]); - } else { + bool ok = false; + param.fRRFPacking = PStringUtils::ToInt(tokens[1], &ok); + if (!ok) error = true; - } } } else { error = true; diff --git a/src/classes/PStringUtils.cpp b/src/classes/PStringUtils.cpp index 573f7e66d..59c05cb88 100644 --- a/src/classes/PStringUtils.cpp +++ b/src/classes/PStringUtils.cpp @@ -28,6 +28,7 @@ ***************************************************************************/ #include +#include #include #include "PStringUtils.h" @@ -125,12 +126,32 @@ bool PStringUtils::IsFloat(const std::string &str) *

Converts the leading part of the string to an int (base 10), mirroring * TString::Atoi(). Returns 0 if no conversion is possible. * + *

Uses std::from_chars so that conversion errors can be reported through + * the optional \a ok out-parameter: it is set to true when a valid integer + * was parsed (leading whitespace is skipped) and to false otherwise (no + * digits present, or the value is out of int range). Trailing non-numeric + * characters are ignored, as with TString::Atoi(). A null \a ok preserves the + * historic fire-and-forget behaviour. + * * \param str string to be converted - * \return converted integer value + * \param ok optional out-parameter signalling conversion success + * \return converted integer value (0 on error) */ -int PStringUtils::ToInt(const std::string &str) +int PStringUtils::ToInt(const std::string &str, bool *ok) { - return static_cast(std::strtol(str.c_str(), nullptr, 10)); + // mirror TString::Atoi(): skip leading whitespace, then parse the leading + // integer (a possible sign followed by decimal digits). + const char *begin = str.c_str(); + const char *end = begin + str.size(); + while (begin != end && std::isspace(static_cast(*begin))) + ++begin; + + int value = 0; + const std::from_chars_result res = std::from_chars(begin, end, value); + if (ok != nullptr) + *ok = (res.ec == std::errc{}); + + return value; } //-------------------------------------------------------------------------- diff --git a/src/include/PStringUtils.h b/src/include/PStringUtils.h index 8bd71cc0a..3cc086c5a 100644 --- a/src/include/PStringUtils.h +++ b/src/include/PStringUtils.h @@ -89,10 +89,16 @@ class PStringUtils *

Converts the leading part of the string to an int (base 10). * Mirrors TString::Atoi(). Returns 0 if no conversion is possible. * + *

If \a ok is non-null it is set to true when a valid integer was + * parsed and to false on error (no digits, or value out of int range). + * This allows callers to distinguish a legitimate 0 from a failed + * conversion, which the bare return value cannot express. + * * @param str string to be converted - * @return converted integer value + * @param ok optional out-parameter signalling conversion success + * @return converted integer value (0 on error) */ - static int ToInt(const std::string &str); + static int ToInt(const std::string &str, bool *ok = nullptr); /** *

Converts the leading part of the string to a double.