From 81346947acba249f1736d786b72c5feb0e43e00e Mon Sep 17 00:00:00 2001 From: Jeremy Lorelli Date: Fri, 11 Aug 2023 17:04:16 -0700 Subject: [PATCH] FIX: incorrect usage of epicsParseInt Consider this function banned in this codebase... bit of a footgun, although it's my fault for not paying attention --- ek9000App/src/devEK9000.cpp | 10 +++++----- ek9000App/src/ekUtil.cpp | 10 +++++----- ek9000App/src/ekUtil.h | 36 +++++++++++++++++++++++++++++++++++- 3 files changed, 45 insertions(+), 11 deletions(-) diff --git a/ek9000App/src/devEK9000.cpp b/ek9000App/src/devEK9000.cpp index 2a73186..1395a89 100644 --- a/ek9000App/src/devEK9000.cpp +++ b/ek9000App/src/devEK9000.cpp @@ -217,7 +217,7 @@ devEK9000Terminal* devEK9000Terminal::ProcessRecordName(const char* recname, int for (size_t i = len; i >= 0; i--) { if (ret[i] == ':' && (size_t)i < len) { ret[i] = '\0'; - if (epicsParseInt32(ret + 1, outindex, 10, NULL) == 0) + if (util::parseNumber(ret + 1, *outindex, 10)) good = true; break; } @@ -1504,16 +1504,16 @@ bool CoE_ParseString(const char* str, ek9k_coe_param_t* param) { else return false; - if (epicsParseInt32(buffers[1], &termid, 10, NULL) != 0) + if (!util::parseNumber(buffers[1], termid, 10)) return 1; pterm = pcoupler->m_terms[termid - 1]; param->pterm = pterm; param->ek9k = pcoupler; - if (epicsParseInt32(buffers[2], ¶m->index, 16, NULL) != 0) + if (!util::parseNumber(buffers[2], param->index, 16)) return 1; - if (epicsParseInt32(buffers[3], ¶m->subindex, 16, NULL) != 0) + if (!util::parseNumber(buffers[3], param->subindex, 16)) return 1; return true; @@ -1720,7 +1720,7 @@ bool ek9k_parse_string(const char* str, ek9k_param_t& param) { } } else if (spec[i].first == "addr") { - if (epicsParseInt32(spec[i].second.c_str(), ¶m.reg, 16, NULL) != 0) { + if (!util::parseNumber(spec[i].second.c_str(), param.reg, 16)) { epicsStdoutPrintf("Malformed integer '%s' in instio string for key 'addr'\n", spec[i].second.c_str()); return false; } diff --git a/ek9000App/src/ekUtil.cpp b/ek9000App/src/ekUtil.cpp index b77cf25..405e471 100644 --- a/ek9000App/src/ekUtil.cpp +++ b/ek9000App/src/ekUtil.cpp @@ -88,8 +88,8 @@ bool util::setupCommonDpvt(const char* recName, const char* inp, TerminalDpvt_t& } /* Terminal position in rail (1=first) */ else if (strcmp(param.first.c_str(), "pos") == 0) { - int term = 0; - bool ok = epicsParseInt32(param.second.c_str(), &term, 10, NULL) != 0; + epicsInt32 term = 0; + bool ok = parseNumber(param.second.c_str(), term, 10); /* Max supported devices by the EK9K is 255 */ if (term < 0 || term > 255 || !ok) { @@ -100,8 +100,8 @@ bool util::setupCommonDpvt(const char* recName, const char* inp, TerminalDpvt_t& } /* Channel number */ else if (strcmp(param.first.c_str(), "channel") == 0) { - int channel = 0; - bool ok = epicsParseInt32(param.second.c_str(), &channel, 10, NULL) != 0; + epicsInt32 channel = 0; + bool ok = parseNumber(param.second.c_str(), channel, 10); /* No real max here, but I think it's good to limit this to 8k as nothing has this many channels */ if (channel < 0 || channel > 8192 || !ok) { epicsPrintf("%s (when parsing %s): invalid channel: %i\n", function, recName, channel); @@ -114,7 +114,7 @@ bool util::setupCommonDpvt(const char* recName, const char* inp, TerminalDpvt_t& const char* tid = param.second.c_str(); if (!strncmp(tid, "EL", 2)) tid += 2; - if (epicsParseInt32(tid, &dpvt.terminalType, 10, NULL) != 0) { + if (parseNumber(tid, dpvt.terminalType, 10) == 0) { epicsPrintf("%s (when parsing %s): unable to parse terminal ID from string '%s'\n", function, recName, param.second.c_str()); return false; diff --git a/ek9000App/src/ekUtil.h b/ek9000App/src/ekUtil.h index 70b4417..c3dd5e4 100644 --- a/ek9000App/src/ekUtil.h +++ b/ek9000App/src/ekUtil.h @@ -56,8 +56,10 @@ #if __cplusplus >= 201703L #define MAYBE_UNUSED [[maybe_unused]] +#define NODISCARD [[nodiscard]] #else #define MAYBE_UNUSED __attribute__((unused)) +#define NODISCARD #endif #undef UNUSED @@ -125,7 +127,7 @@ class FmtStr { FmtStr(const char* fmt, ...) EPICS_PRINTF_STYLE(2, 3) { va_list va; va_start(va, fmt); - vsnprintf(str_, sizeof(str_), fmt, va); + (void)vsnprintf(str_, sizeof(str_), fmt, va); va_end(va); } @@ -200,9 +202,14 @@ concept INPUT_RECORD = requires(T a) { template concept RECORD_TYPE = detail::BASE_RECORD &&(detail::INPUT_RECORD || detail::OUTPUT_RECORD); +template +concept NUMERIC_TYPE = std::same_as || std::same_as || std::same_as || + std::same_as || std::same_as || std::same_as; + #else #define RECORD_TYPE typename +#define NUMERIC_TYPE typename #endif @@ -259,7 +266,34 @@ template <> inline bool setupCommonDpvt(aoRecord* prec, TerminalDpvt_t return setupCommonDpvt(prec->name, prec->out.value.instio.string, dpvt); } +template NODISCARD inline bool parseNumber(const char* str, T& out, int base = 10); + +template <> NODISCARD inline bool parseNumber(const char* str, epicsInt16& out, int base) { + return epicsParseInt16(str, &out, base, NULL) == 0; +} + +template <> NODISCARD inline bool parseNumber(const char* str, epicsUInt16& out, int base) { + return epicsParseUInt16(str, &out, base, NULL) == 0; +} + +template <> NODISCARD inline bool parseNumber(const char* str, epicsInt32& out, int base) { + return epicsParseInt32(str, &out, base, NULL) == 0; +} + +template <> NODISCARD inline bool parseNumber(const char* str, epicsUInt32& out, int base) { + return epicsParseUInt32(str, &out, base, NULL) == 0; +} + +template <> NODISCARD inline bool parseNumber(const char* str, epicsInt64& out, int base) { + return epicsParseInt64(str, &out, base, NULL) == 0; +} + +template <> NODISCARD inline bool parseNumber(const char* str, epicsUInt64& out, int base) { + return epicsParseUInt64(str, &out, base, NULL) == 0; +} + } // namespace util // Clear pre-C++20 concept hacks #undef RECORD_TYPE +#undef NUMERIC_TYPE