From: Gerhard Sittig Date: Thu, 19 Jul 2018 19:17:16 +0000 (+0200) Subject: strutil: avoid glib/platform conversion calls for empty input X-Git-Url: https://sigrok.org/gitaction?a=commitdiff_plain;h=ca9949980b146606a8f61f926004d1a6fb27a2a7;p=libsigrok.git strutil: avoid glib/platform conversion calls for empty input It appears that either glib or some underlying platform library code for text to number conversion yields unexpected values for return codes, errno, and "end pointer" when _none_ of the input could get converted. This was reported for MacOS, did not reproduce on Linux. The reported issue was addressed in commit 51bf39a1633c, this implementation is just much more conservative and hopefully easier to verify since it's more explicit. Rephrase the rational text to number conversion to avoid the problematic input cases. Address style nits while we are here in the hope to improve readability: Move initial assignment and subsequent updates of variables closer together for easier verification. Accept optional leading whitespace. Accept numbers like "123." in addition to the previously supported ".123" formats. But insist that one of the integral or fractional parts must be present. Try harder to setup non-zero errno values for other failed conversion attempts beyond failed library calls (the strtol(3) manpage suggests so in the ERRORS section). This implementation passes on Linux, but has not been tested with MacOS which appears to have a pickier library implementation. --- diff --git a/src/strutil.c b/src/strutil.c index c22c7175..d704de45 100644 --- a/src/strutil.c +++ b/src/strutil.c @@ -828,78 +828,149 @@ SR_PRIV void sr_hexdump_free(GString *s) */ SR_API int sr_parse_rational(const char *str, struct sr_rational *ret) { - char *endptr = NULL; + const char *readptr; + char *endptr; + gboolean is_negative, empty_integral, empty_fractional, exp_negative; int64_t integral; - int64_t fractional = 0; - int64_t denominator = 1; - int32_t fractional_len = 0; - int32_t exponent = 0; - gboolean is_negative = FALSE; - gboolean no_integer, no_fractional; - - while (isspace(*str)) - str++; + int64_t fractional; + int64_t denominator; + uint32_t fractional_len; + int32_t exponent; - errno = 0; - integral = g_ascii_strtoll(str, &endptr, 10); - - if (str == endptr && (str[0] == '-' || str[0] == '+') && str[1] == '.') { - endptr += 1; - no_integer = TRUE; - } else if (str == endptr && str[0] == '.') { - no_integer = TRUE; - } else if (errno) { - return SR_ERR; - } else { - no_integer = FALSE; - } + /* + * Implementor's note: This routine tries hard to avoid calling + * glib's or the platform's conversion routines with input that + * cannot get converted *at all* (see bug #1093). It also takes + * care to return with non-zero errno values for any failed + * conversion attempt. It's assumed that correctness and robustness + * are more important than performance, which is why code paths + * are not optimized at all. Maintainability took priority. + */ + + readptr = str; - if (integral < 0 || str[0] == '-') + /* Skip leading whitespace. */ + while (isspace(*readptr)) + readptr++; + + /* Determine the sign, default to non-negative. */ + is_negative = FALSE; + if (*readptr == '-') { is_negative = TRUE; + readptr++; + } else if (*readptr == '+') { + is_negative = FALSE; + readptr++; + } + /* Get the (optional) integral part. */ + empty_integral = TRUE; + integral = 0; + endptr = (char *)readptr; errno = 0; - if (*endptr == '.') { - gboolean is_exp, is_eos; - const char *start = endptr + 1; - fractional = g_ascii_strtoll(start, &endptr, 10); - is_exp = *endptr == 'E' || *endptr == 'e'; - is_eos = *endptr == '\0'; - if (endptr == start && (is_exp || is_eos)) { - fractional = 0; - errno = 0; - } + if (isdigit(*readptr)) { + empty_integral = FALSE; + integral = g_ascii_strtoll(readptr, &endptr, 10); if (errno) return SR_ERR; - no_fractional = endptr == start; - if (no_integer && no_fractional) + if (endptr == str) { + errno = -EINVAL; return SR_ERR; - fractional_len = endptr - start; + } + readptr = endptr; } - errno = 0; - if ((*endptr == 'E') || (*endptr == 'e')) { - exponent = g_ascii_strtoll(endptr + 1, &endptr, 10); + /* Get the optional fractional part. */ + empty_fractional = TRUE; + fractional = 0; + fractional_len = 0; + if (*readptr == '.') { + readptr++; + endptr++; + errno = 0; + if (isdigit(*readptr)) { + empty_fractional = FALSE; + fractional = g_ascii_strtoll(readptr, &endptr, 10); + if (errno) + return SR_ERR; + if (endptr == readptr) { + errno = -EINVAL; + return SR_ERR; + } + fractional_len = endptr - readptr; + readptr = endptr; + } + } + + /* At least one of integral or fractional is required. */ + if (empty_integral && empty_fractional) { + errno = -EINVAL; + return SR_ERR; + } + + /* Get the (optional) exponent. */ + exponent = 0; + if ((*readptr == 'E') || (*readptr == 'e')) { + readptr++; + endptr++; + exp_negative = FALSE; + if (*readptr == '+') { + exp_negative = FALSE; + readptr++; + endptr++; + } else if (*readptr == '-') { + exp_negative = TRUE; + readptr++; + endptr++; + } + if (!isdigit(*readptr)) { + errno = -EINVAL; + return SR_ERR; + } + errno = 0; + exponent = g_ascii_strtoll(readptr, &endptr, 10); if (errno) return SR_ERR; + if (endptr == readptr) { + errno = -EINVAL; + return SR_ERR; + } + readptr = endptr; + if (exp_negative) + exponent = -exponent; } - if (*endptr != '\0') + /* Input must be exhausted. Unconverted remaining input is fatal. */ + if (*endptr != '\0') { + errno = -EINVAL; return SR_ERR; + } - for (int i = 0; i < fractional_len; i++) + /* + * Apply the sign to the integral (and fractional) part(s). + * Adjust exponent (decimal position) such that the above integral + * and fractional parts both fit into the (new) integral part. + */ + if (is_negative) + integral = -integral; + while (fractional_len-- > 0) { integral *= 10; - exponent -= fractional_len; - + exponent--; + } if (!is_negative) integral += fractional; else integral -= fractional; - while (exponent > 0) { integral *= 10; exponent--; } + /* + * When significant digits remain after the decimal, scale up the + * denominator such that we end up with two integer p/q numbers. + */ + denominator = 1; while (exponent < 0) { denominator *= 10; exponent++;