Skip to content

Commit

Permalink
FIX: incorrect usage of epicsParseInt
Browse files Browse the repository at this point in the history
Consider this function banned in this codebase... bit of a footgun, although it's my fault for not paying attention
  • Loading branch information
JJL772 committed Aug 14, 2023
1 parent 9749c6c commit 8134694
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 11 deletions.
10 changes: 5 additions & 5 deletions ek9000App/src/devEK9000.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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], &param->index, 16, NULL) != 0)
if (!util::parseNumber(buffers[2], param->index, 16))
return 1;

if (epicsParseInt32(buffers[3], &param->subindex, 16, NULL) != 0)
if (!util::parseNumber(buffers[3], param->subindex, 16))
return 1;

return true;
Expand Down Expand Up @@ -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(), &param.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;
}
Expand Down
10 changes: 5 additions & 5 deletions ek9000App/src/ekUtil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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);
Expand All @@ -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;
Expand Down
36 changes: 35 additions & 1 deletion ek9000App/src/ekUtil.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -200,9 +202,14 @@ concept INPUT_RECORD = requires(T a) {
template <typename T>
concept RECORD_TYPE = detail::BASE_RECORD<T> &&(detail::INPUT_RECORD<T> || detail::OUTPUT_RECORD<T>);

template <typename T>
concept NUMERIC_TYPE = std::same_as<T, epicsInt32> || std::same_as<T, epicsUInt32> || std::same_as<T, epicsInt16> ||
std::same_as<T, epicsUInt16> || std::same_as<T, epicsInt64> || std::same_as<T, epicsUInt64>;

#else

#define RECORD_TYPE typename
#define NUMERIC_TYPE typename

#endif

Expand Down Expand Up @@ -259,7 +266,34 @@ template <> inline bool setupCommonDpvt<aoRecord>(aoRecord* prec, TerminalDpvt_t
return setupCommonDpvt(prec->name, prec->out.value.instio.string, dpvt);
}

template <NUMERIC_TYPE T> NODISCARD inline bool parseNumber(const char* str, T& out, int base = 10);

template <> NODISCARD inline bool parseNumber<epicsInt16>(const char* str, epicsInt16& out, int base) {
return epicsParseInt16(str, &out, base, NULL) == 0;
}

template <> NODISCARD inline bool parseNumber<epicsUInt16>(const char* str, epicsUInt16& out, int base) {
return epicsParseUInt16(str, &out, base, NULL) == 0;
}

template <> NODISCARD inline bool parseNumber<epicsInt32>(const char* str, epicsInt32& out, int base) {
return epicsParseInt32(str, &out, base, NULL) == 0;
}

template <> NODISCARD inline bool parseNumber<epicsUInt32>(const char* str, epicsUInt32& out, int base) {
return epicsParseUInt32(str, &out, base, NULL) == 0;
}

template <> NODISCARD inline bool parseNumber<epicsInt64>(const char* str, epicsInt64& out, int base) {
return epicsParseInt64(str, &out, base, NULL) == 0;
}

template <> NODISCARD inline bool parseNumber<epicsUInt64>(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

0 comments on commit 8134694

Please sign in to comment.