From 20b8c9700a0a81abb56703add03411eb40dc9a88 Mon Sep 17 00:00:00 2001 From: Andrew Wood Date: Sat, 16 Sep 2023 00:55:40 +0100 Subject: [PATCH] Rename the functions to better describe what they do, remove single-letter variable names, add more explanatory comments, remove "_t" suffix from typedef as per FreeBSD style guide. --- src/include/pv.h | 24 +++++---- src/main/options.c | 18 +++---- src/pv/cursor.c | 2 +- src/pv/number.c | 132 ++++++++++++++++++++++++++++++--------------- 4 files changed, 111 insertions(+), 65 deletions(-) diff --git a/src/include/pv.h b/src/include/pv.h index f1380a3..0e651b5 100644 --- a/src/include/pv.h +++ b/src/include/pv.h @@ -34,7 +34,7 @@ typedef struct pvstate_s *pvstate_t; typedef enum { PV_NUMTYPE_INTEGER, PV_NUMTYPE_DOUBLE -} pv_numtype_t; +} pv_numtype; /* @@ -42,25 +42,27 @@ typedef enum { */ /* - * Return the given string converted to a double. + * Return the given string converted to a double, for use as a time + * interval. */ -extern double pv_getnum_d(const char *); +extern double pv_getnum_interval(const char *); /* - * Return the given string converted to an unsigned integer. - */ -extern unsigned int pv_getnum_ui(const char *); - -/* - * Return the given string converted to an off_t. + * Return the given string converted to an off_t, for use as a size. */ extern off_t pv_getnum_size(const char *); /* - * Return zero if the given string is a number of the given type. NB an + * Return the given string converted to an unsigned integer, for use as a + * count such as screen width. + */ +extern unsigned int pv_getnum_count(const char *); + +/* + * Return true if the given string is a number of the given type. NB an * integer is both a valid integer and a valid double. */ -extern int pv_getnum_check(const char *, pv_numtype_t); +extern bool pv_getnum_check(const char *, pv_numtype); /* * String handling wrappers. diff --git a/src/main/options.c b/src/main/options.c index 502c9c1..2a3c8be 100644 --- a/src/main/options.c +++ b/src/main/options.c @@ -249,7 +249,7 @@ opts_t opts_parse(unsigned int argc, char **argv) case 'm': /*@fallthrough@ */ case 'Z': - if (pv_getnum_check(optarg, PV_NUMTYPE_INTEGER) != 0) { + if (!pv_getnum_check(optarg, PV_NUMTYPE_INTEGER)) { /*@-mustfreefresh@ *//* see above */ fprintf(stderr, "%s: -%c: %s\n", opts->program_name, c, _("integer argument expected")); opts_free(opts); @@ -260,7 +260,7 @@ opts_t opts_parse(unsigned int argc, char **argv) case 'i': /*@fallthrough@ */ case 'D': - if (pv_getnum_check(optarg, PV_NUMTYPE_DOUBLE) != 0) { + if (!pv_getnum_check(optarg, PV_NUMTYPE_DOUBLE)) { /*@-mustfreefresh@ *//* see above */ fprintf(stderr, "%s: -%c: %s\n", opts->program_name, c, _("numeric argument expected")); opts_free(opts); @@ -341,7 +341,7 @@ opts_t opts_parse(unsigned int argc, char **argv) opts->no_splice = true; break; case 'A': - opts->lastwritten = (size_t) pv_getnum_ui(optarg); + opts->lastwritten = (size_t) pv_getnum_count(optarg); numopts++; opts->no_splice = true; break; @@ -363,7 +363,7 @@ opts_t opts_parse(unsigned int argc, char **argv) opts->wait = true; break; case 'D': - opts->delay_start = pv_getnum_d(optarg); + opts->delay_start = pv_getnum_interval(optarg); break; case 's': /* Permit "@" as well as just a number. */ @@ -398,14 +398,14 @@ opts_t opts_parse(unsigned int argc, char **argv) opts->linemode = true; break; case 'i': - opts->interval = pv_getnum_d(optarg); + opts->interval = pv_getnum_interval(optarg); break; case 'w': - opts->width = pv_getnum_ui(optarg); + opts->width = pv_getnum_count(optarg); opts->width_set_manually = opts->width == 0 ? false : true; break; case 'H': - opts->height = pv_getnum_ui(optarg); + opts->height = pv_getnum_count(optarg); opts->height_set_manually = opts->height == 0 ? false : true; break; case 'N': @@ -446,7 +446,7 @@ opts_t opts_parse(unsigned int argc, char **argv) opts->no_splice = true; break; case 'R': - opts->remote = pv_getnum_ui(optarg); + opts->remote = pv_getnum_count(optarg); break; case 'P': opts->pidfile = pv_strdup(optarg); @@ -471,7 +471,7 @@ opts_t opts_parse(unsigned int argc, char **argv) (void) sscanf(optarg, "%u:%d", &(opts->watch_pid), &(opts->watch_fd)); break; case 'm': - opts->average_rate_window = pv_getnum_ui(optarg); + opts->average_rate_window = pv_getnum_count(optarg); break; #ifdef ENABLE_DEBUGGING case '!': diff --git a/src/pv/cursor.c b/src/pv/cursor.c index ec986cf..2618304 100644 --- a/src/pv/cursor.c +++ b/src/pv/cursor.c @@ -293,7 +293,7 @@ static int pv_crs_get_ypos(int terminalfd) } #endif /* !CURSOR_ANSWERBACK_BYTE_BY_BYTE */ - ypos = (int) pv_getnum_ui(cpr + 2); + ypos = (int) pv_getnum_count(cpr + 2); if (0 != tcsetattr(terminalfd, TCSANOW | TCSAFLUSH, &old_tty)) { debug("%s: %s", "tcsetattr (2) failed", strerror(errno)); diff --git a/src/pv/number.c b/src/pv/number.c index c6cebfc..e62f9be 100644 --- a/src/pv/number.c +++ b/src/pv/number.c @@ -24,36 +24,50 @@ static bool pv__isdigit(char c) /* - * Return the numeric value of "str", as an off_t. + * Return the numeric value of "str", as an off_t, where "str" is expected + * to be a sequence of digits (without a thousands separator), possibly with + * a fractional part, optionally followed by a units suffix such as "K" for + * kibibytes. */ off_t pv_getnum_size(const char *str) { - off_t n = 0; - off_t decimal = 0; - unsigned int decdivisor = 1; + off_t integral_part = 0; + off_t fractional_part = 0; + unsigned int fractional_divisor = 1; unsigned int shift = 0; if (NULL == str) - return n; + return (off_t) 0; + /* Skip any non-numeric leading characters. */ while (str[0] != '\0' && (!pv__isdigit(str[0]))) str++; + /* + * Parse the integral part of the number - the digits before the + * decimal mark or units. + */ for (; pv__isdigit(str[0]); str++) { - n = n * 10; - n += (off_t) (str[0] - '0'); + integral_part = integral_part * 10; + integral_part += (off_t) (str[0] - '0'); } /* - * If a decimal value was given, skip the decimal part. + * If the next character is a decimal mark, skip over it and parse + * the following digits as the fractional part of the number. + * + * Note that we hard-code the decimal mark as '.' or ',' so this + * will fail if there are any locales whose decimal mark is not one + * of those two characters. */ if (('.' == str[0]) || (',' == str[0])) { str++; for (; pv__isdigit(str[0]); str++) { - if (decdivisor < 10000) { - decimal = decimal * 10; - decimal += (off_t) (str[0] - '0'); - decdivisor = decdivisor * 10; + /* Stop counting below 0.0001. */ + if (fractional_divisor < 10000) { + fractional_part = fractional_part * 10; + fractional_part += (off_t) (str[0] - '0'); + fractional_divisor = fractional_divisor * 10; } } } @@ -63,6 +77,7 @@ off_t pv_getnum_size(const char *str) * T=TiB=1024GiB). */ if (str[0] != '\0') { + /* Skip any spaces or tabs after the digits. */ while ((' ' == str[0]) || ('\t' == str[0])) str++; switch (str[0]) { @@ -99,98 +114,126 @@ off_t pv_getnum_size(const char *str) if (shiftby > 30) shiftby = 30; - n = (off_t) (n << shiftby); - decimal = (off_t) (decimal << shiftby); + /*@-shiftimplementation@*/ + /* + * splint note: ignore the fact that the types we are + * shifting are signed, because we know they are definitely + * not negative. + */ + integral_part = (off_t) (integral_part << shiftby); + fractional_part = (off_t) (fractional_part << shiftby); + /*@+shiftimplementation@*/ + shift -= shiftby; } /* - * Add any decimal component. + * Add the fractional part, divided by its divisor, to the integral + * part, now that we've multiplied everything by the appropriate + * units. */ - decimal = decimal / decdivisor; - n += decimal; + fractional_part = fractional_part / fractional_divisor; + integral_part += fractional_part; - return n; + return integral_part; } /* - * Return the numeric value of "str", as a double. + * Return the numeric value of "str", as a double, where "str" is expected + * to be a positive decimal number expressing a time interval. */ -double pv_getnum_d(const char *str) +double pv_getnum_interval(const char *str) { - double n = 0.0; + double result = 0.0; double step = 1; if (NULL == str) - return n; + return 0.0; + /* Skip any non-digit characters at the start. */ while (str[0] != '\0' && (!pv__isdigit(str[0]))) str++; + /* Parse the digits before the decimal mark. */ for (; pv__isdigit(str[0]); str++) { - n = n * 10; - n += (double) (str[0] - '0'); + result = result * 10; + result += (double) (str[0] - '0'); } + /* If there is no decimal mark, return the value as-is. */ if ((str[0] != '.') && (str[0] != ',')) - return n; + return result; + /* Move past the decimal mark. */ str++; + /* Parse the digits after the decimal mark, up to 0.0000001. */ for (; pv__isdigit(str[0]) && step < 1000000; str++) { step = step * 10; - n += ((double) (str[0] - '0')) / step; + result += ((double) (str[0] - '0')) / step; } - return n; + return result; } /* - * Return the numeric value of "str", as an unsigned int. + * Return the numeric value of "str", as an unsigned int, following the same + * rules as pv_getnum_size(), expecting "str" to express a value to be used + * as a count (such as number of screen columns, or size of a buffer). */ -unsigned int pv_getnum_ui(const char *str) +unsigned int pv_getnum_count(const char *str) { return (unsigned int) pv_getnum_size(str); } /* - * Return nonzero if the given string is not a valid number of the given - * type. + * Return true if the given string is a valid number of the given type. */ -int pv_getnum_check(const char *str, pv_numtype_t type) +bool pv_getnum_check(const char *str, pv_numtype type) { - if (0 == str) - return 1; + if (NULL == str) + return false; + /* Skip leading spaces and tabs. */ while ((' ' == str[0]) || ('\t' == str[0])) str++; + /* If the next character isn't a digit, this isn't a number. */ if (!pv__isdigit(str[0])) - return 1; + return false; + /* Skip over the digits. */ for (; pv__isdigit(str[0]); str++); + /* + * If there's a decimal mark (see note in pv_getnum_size() above), + * check that too. + */ if (('.' == str[0]) || (',' == str[0])) { + /* Integers should have no decimal mark. */ if (type == PV_NUMTYPE_INTEGER) - return 1; + return false; + /* Skip the decimal mark, then all digits. */ str++; for (; pv__isdigit(str[0]); str++); } + /* If the string ends here, this is a valid number. */ if ('\0' == str[0]) - return 0; + return true; - /* - * Suffixes are not allowed for doubles, only for integers. - */ + /* A units suffix is not allowed for doubles, only for integers. */ if (type == PV_NUMTYPE_DOUBLE) - return 1; + return false; + /* Skip trailing spaces or tabs. */ while ((' ' == str[0]) || ('\t' == str[0])) str++; + + /* Check the units suffix is one we know about. */ switch (str[0]) { case 'k': case 'K': @@ -203,13 +246,14 @@ int pv_getnum_check(const char *str, pv_numtype_t type) str++; break; default: - return 1; + return false; } + /* If the string has trailing text, it's not a valid number. */ if (str[0] != '\0') - return 1; + return false; - return 0; + return true; } /* EOF */