From 61274a7cdbab8b921171c4d2fa5b8c3c75bde311 Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Sat, 7 Dec 2019 11:01:55 +0100 Subject: [PATCH] Factorize integer argument parsing Add util functions for integer parsing (with tests), and factorize integer argument parsing to avoid code duplication. --- app/src/main.c | 138 ++++++++++++++------------------------- app/src/util/str_util.c | 55 ++++++++++++++++ app/src/util/str_util.h | 13 ++++ app/tests/test_strutil.c | 71 ++++++++++++++++++++ 4 files changed, 189 insertions(+), 88 deletions(-) diff --git a/app/src/main.c b/app/src/main.c index e9a2b9aa..53ef253e 100644 --- a/app/src/main.c +++ b/app/src/main.c @@ -12,6 +12,7 @@ #include "compat.h" #include "recorder.h" #include "util/log.h" +#include "util/str_util.h" struct args { struct scrcpy_options opts; @@ -224,51 +225,47 @@ print_version(void) { } static bool -parse_bit_rate(char *optarg, uint32_t *bit_rate) { - char *endptr; - if (*optarg == '\0') { - LOGE("Bit-rate parameter is empty"); - return false; +parse_integer_arg(const char *s, long *out, bool accept_suffix, long min, + long max, const char *name) { + long value; + bool ok; + if (accept_suffix) { + ok = parse_integer_with_suffix(s, &value); + } else { + ok = parse_integer(s, &value); } - long value = strtol(optarg, &endptr, 0); - int mul = 1; - if (*endptr != '\0') { - if (optarg == endptr) { - LOGE("Invalid bit-rate: %s", optarg); - return false; - } - if ((*endptr == 'M' || *endptr == 'm') && endptr[1] == '\0') { - mul = 1000000; - } else if ((*endptr == 'K' || *endptr == 'k') && endptr[1] == '\0') { - mul = 1000; - } else { - LOGE("Invalid bit-rate unit: %s", optarg); - return false; - } - } - if (value < 0 || ((uint32_t) -1) / mul < (unsigned long) value) { - LOGE("Bitrate must be positive and less than 2^32: %s", optarg); + if (!ok) { + LOGE("Could not parse %s: %s", name, s); return false; } - *bit_rate = (uint32_t) value * mul; + if (value < min || value > max) { + LOGE("Could not parse %s: value (%ld) out-of-range (%ld; %ld)", + name, value, min, max); + return false; + } + + *out = value; return true; } static bool -parse_max_size(char *optarg, uint16_t *max_size) { - char *endptr; - if (*optarg == '\0') { - LOGE("Max size parameter is empty"); +parse_bit_rate(const char *s, uint32_t *bit_rate) { + long value; + bool ok = parse_integer_arg(s, &value, true, 0, 0xFFFF, "bit-rate"); + if (!ok) { return false; } - long value = strtol(optarg, &endptr, 0); - if (*endptr != '\0') { - LOGE("Invalid max size: %s", optarg); - return false; - } - if (value & ~0xffff) { - LOGE("Max size must be between 0 and 65535: %ld", value); + + *bit_rate = (uint32_t) value; + return true; +} + +static bool +parse_max_size(char *s, uint16_t *max_size) { + long value; + bool ok = parse_integer_arg(s, &value, false, 0, 0xFFFF, "max size"); + if (!ok) { return false; } @@ -277,20 +274,10 @@ parse_max_size(char *optarg, uint16_t *max_size) { } static bool -parse_max_fps(const char *optarg, uint16_t *max_fps) { - char *endptr; - if (*optarg == '\0') { - LOGE("Max FPS parameter is empty"); - return false; - } - long value = strtol(optarg, &endptr, 0); - if (*endptr != '\0') { - LOGE("Invalid max FPS: %s", optarg); - return false; - } - if (value & ~0xffff) { - // in practice, it should not be higher than 60 - LOGE("Max FPS value is invalid: %ld", value); +parse_max_fps(const char *s, uint16_t *max_fps) { + long value; + bool ok = parse_integer_arg(s, &value, false, 0, 1000, "max fps"); + if (!ok) { return false; } @@ -299,19 +286,11 @@ parse_max_fps(const char *optarg, uint16_t *max_fps) { } static bool -parse_window_position(char *optarg, int16_t *position) { - char *endptr; - if (*optarg == '\0') { - LOGE("Window position parameter is empty"); - return false; - } - long value = strtol(optarg, &endptr, 0); - if (*endptr != '\0') { - LOGE("Invalid window position: %s", optarg); - return false; - } - if (value < -1 || value > 0x7fff) { - LOGE("Window position must be between -1 and 32767: %ld", value); +parse_window_position(char *s, int16_t *position) { + long value; + bool ok = parse_integer_arg(s, &value, false, -1, 0x7FFF, + "window position"); + if (!ok) { return false; } @@ -320,19 +299,11 @@ parse_window_position(char *optarg, int16_t *position) { } static bool -parse_window_dimension(char *optarg, uint16_t *dimension) { - char *endptr; - if (*optarg == '\0') { - LOGE("Window dimension parameter is empty"); - return false; - } - long value = strtol(optarg, &endptr, 0); - if (*endptr != '\0') { - LOGE("Invalid window dimension: %s", optarg); - return false; - } - if (value & ~0xffff) { - LOGE("Window position must be between 0 and 65535: %ld", value); +parse_window_dimension(char *s, uint16_t *dimension) { + long value; + bool ok = parse_integer_arg(s, &value, false, 0, 0xFFFF, + "window dimension"); + if (!ok) { return false; } @@ -341,19 +312,10 @@ parse_window_dimension(char *optarg, uint16_t *dimension) { } static bool -parse_port(char *optarg, uint16_t *port) { - char *endptr; - if (*optarg == '\0') { - LOGE("Port parameter is empty"); - return false; - } - long value = strtol(optarg, &endptr, 0); - if (*endptr != '\0') { - LOGE("Invalid port: %s", optarg); - return false; - } - if (value & ~0xffff) { - LOGE("Port out of range: %ld", value); +parse_port(char *s, uint16_t *port) { + long value; + bool ok = parse_integer_arg(s, &value, false, 0, 0xFFFF, "port"); + if (!ok) { return false; } diff --git a/app/src/util/str_util.c b/app/src/util/str_util.c index 15378d8a..4d175407 100644 --- a/app/src/util/str_util.c +++ b/app/src/util/str_util.c @@ -1,5 +1,7 @@ #include "str_util.h" +#include +#include #include #include @@ -60,6 +62,59 @@ strquote(const char *src) { return quoted; } +bool +parse_integer(const char *s, long *out) { + char *endptr; + if (*s == '\0') { + return false; + } + errno = 0; + long value = strtol(s, &endptr, 0); + if (errno == ERANGE) { + return false; + } + if (*endptr != '\0') { + return false; + } + + *out = value; + return true; +} + +bool +parse_integer_with_suffix(const char *s, long *out) { + char *endptr; + if (*s == '\0') { + return false; + } + errno = 0; + long value = strtol(s, &endptr, 0); + if (errno == ERANGE) { + return false; + } + int mul = 1; + if (*endptr != '\0') { + if (s == endptr) { + return false; + } + if ((*endptr == 'M' || *endptr == 'm') && endptr[1] == '\0') { + mul = 1000000; + } else if ((*endptr == 'K' || *endptr == 'k') && endptr[1] == '\0') { + mul = 1000; + } else { + return false; + } + } + + if ((value < 0 && LONG_MIN / mul > value) || + (value > 0 && LONG_MAX / mul < value)) { + return false; + } + + *out = value * mul; + return true; +} + size_t utf8_truncation_index(const char *utf8, size_t max_len) { size_t len = strlen(utf8); diff --git a/app/src/util/str_util.h b/app/src/util/str_util.h index 56490190..8d9b990c 100644 --- a/app/src/util/str_util.h +++ b/app/src/util/str_util.h @@ -1,6 +1,7 @@ #ifndef STRUTIL_H #define STRUTIL_H +#include #include #include "config.h" @@ -25,6 +26,18 @@ xstrjoin(char *dst, const char *const tokens[], char sep, size_t n); char * strquote(const char *src); +// parse s as an integer into value +// returns true if the conversion succeeded, false otherwise +bool +parse_integer(const char *s, long *out); + +// parse s as an integer into value +// like parse_integer(), but accept 'k'/'K' (x1000) and 'm'/'M' (x1000000) as +// suffix +// returns true if the conversion succeeded, false otherwise +bool +parse_integer_with_suffix(const char *s, long *out); + // return the index to truncate a UTF-8 string at a valid position size_t utf8_truncation_index(const char *utf8, size_t max_len); diff --git a/app/tests/test_strutil.c b/app/tests/test_strutil.c index ddc8399b..baa2fd38 100644 --- a/app/tests/test_strutil.c +++ b/app/tests/test_strutil.c @@ -1,4 +1,6 @@ #include +#include +#include #include #define SDL_MAIN_HANDLED // avoid to redefine main to SDL_main #include @@ -169,6 +171,73 @@ static void test_utf8_truncate(void) { assert(count == 7); // no more chars } +static void test_parse_integer(void) { + long value; + bool ok = parse_integer("1234", &value); + assert(ok); + assert(value == 1234); + + ok = parse_integer("-1234", &value); + assert(ok); + assert(value == -1234); + + ok = parse_integer("1234k", &value); + assert(!ok); + + ok = parse_integer("123456789876543212345678987654321", &value); + assert(!ok); // out-of-range +} + +static void test_parse_integer_with_suffix(void) { + long value; + bool ok = parse_integer_with_suffix("1234", &value); + assert(ok); + assert(value == 1234); + + ok = parse_integer_with_suffix("-1234", &value); + assert(ok); + assert(value == -1234); + + ok = parse_integer_with_suffix("1234k", &value); + assert(ok); + assert(value == 1234000); + + ok = parse_integer_with_suffix("1234m", &value); + assert(ok); + assert(value == 1234000000); + + ok = parse_integer_with_suffix("-1234k", &value); + assert(ok); + assert(value == -1234000); + + ok = parse_integer_with_suffix("-1234m", &value); + assert(ok); + assert(value == -1234000000); + + ok = parse_integer_with_suffix("123456789876543212345678987654321", &value); + assert(!ok); // out-of-range + + char buf[32]; + + sprintf(buf, "%ldk", LONG_MAX / 2000); + ok = parse_integer_with_suffix(buf, &value); + assert(ok); + assert(value == LONG_MAX / 2000 * 1000); + + sprintf(buf, "%ldm", LONG_MAX / 2000); + ok = parse_integer_with_suffix(buf, &value); + assert(!ok); + + sprintf(buf, "%ldk", LONG_MIN / 2000); + ok = parse_integer_with_suffix(buf, &value); + assert(ok); + assert(value == LONG_MIN / 2000 * 1000); + + sprintf(buf, "%ldm", LONG_MIN / 2000); + ok = parse_integer_with_suffix(buf, &value); + assert(!ok); +} + int main(void) { test_xstrncpy_simple(); test_xstrncpy_just_fit(); @@ -180,5 +249,7 @@ int main(void) { test_xstrjoin_truncated_after_sep(); test_strquote(); test_utf8_truncate(); + test_parse_integer(); + test_parse_integer_with_suffix(); return 0; }