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

@@ -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) {
+parse_integer_arg(const char *s, long *out, bool accept_suffix, long min,
-    char *endptr;
+                  long max, const char *name) {
-    if (*optarg == '\0') {
+    long value;
-        LOGE("Bit-rate parameter is empty");
+    bool ok;
-        return false;
+    if (accept_suffix) {
-    }
+        ok = parse_integer_with_suffix(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);
+        ok = parse_integer(s, &value);
-            return false;
     }
-    }
+    if (!ok) {
-    if (value < 0 || ((uint32_t) -1) / mul < (unsigned long) value) {
+        LOGE("Could not parse %s: %s", name, s);
-        LOGE("Bitrate must be positive and less than 2^32: %s", optarg);
         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) {
+parse_bit_rate(const char *s, uint32_t *bit_rate) {
-    char *endptr;
+    long value;
-    if (*optarg == '\0') {
+    bool ok = parse_integer_arg(s, &value, true, 0, 0xFFFF, "bit-rate");
-        LOGE("Max size parameter is empty");
+    if (!ok) {
         return false;
     }
-    long value = strtol(optarg, &endptr, 0);
+
-    if (*endptr != '\0') {
+    *bit_rate = (uint32_t) value;
-        LOGE("Invalid max size: %s", optarg);
+    return true;
-        return false;
 }
-    if (value & ~0xffff) {
+
-        LOGE("Max size must be between 0 and 65535: %ld", value);
+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) {
+parse_max_fps(const char *s, uint16_t *max_fps) {
-    char *endptr;
+    long value;
-    if (*optarg == '\0') {
+    bool ok = parse_integer_arg(s, &value, false, 0, 1000, "max fps");
-        LOGE("Max FPS parameter is empty");
+    if (!ok) {
-        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);
         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) {
+parse_window_position(char *s, int16_t *position) {
-    char *endptr;
+    long value;
-    if (*optarg == '\0') {
+    bool ok = parse_integer_arg(s, &value, false, -1, 0x7FFF,
-        LOGE("Window position parameter is empty");
+                                "window position");
-        return false;
+    if (!ok) {
-    }
-    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);
         return false;
     }
 
@@ -320,19 +299,11 @@ parse_window_position(char *optarg, int16_t *position) {
 }
 
 static bool
-parse_window_dimension(char *optarg, uint16_t *dimension) {
+parse_window_dimension(char *s, uint16_t *dimension) {
-    char *endptr;
+    long value;
-    if (*optarg == '\0') {
+    bool ok = parse_integer_arg(s, &value, false, 0, 0xFFFF,
-        LOGE("Window dimension parameter is empty");
+                                "window dimension");
-        return false;
+    if (!ok) {
-    }
-    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);
         return false;
     }
 
@@ -341,19 +312,10 @@ parse_window_dimension(char *optarg, uint16_t *dimension) {
 }
 
 static bool
-parse_port(char *optarg, uint16_t *port) {
+parse_port(char *s, uint16_t *port) {
-    char *endptr;
+    long value;
-    if (*optarg == '\0') {
+    bool ok = parse_integer_arg(s, &value, false, 0, 0xFFFF, "port");
-        LOGE("Port parameter is empty");
+    if (!ok) {
-        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);
         return false;
     }
 

app/src/util/str_util.c

@@ -1,5 +1,7 @@
 #include "str_util.h"
 
+#include <errno.h>
+#include <limits.h>
 #include <stdlib.h>
 #include <string.h>
 
@@ -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);

app/src/util/str_util.h

@@ -1,6 +1,7 @@
 #ifndef STRUTIL_H
 #define STRUTIL_H
 
+#include <stdbool.h>
 #include <stddef.h>
 
 #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);

app/tests/test_strutil.c

@@ -1,4 +1,6 @@
 #include <assert.h>
+#include <limits.h>
+#include <stdio.h>
 #include <string.h>
 #define SDL_MAIN_HANDLED // avoid to redefine main to SDL_main
 #include <SDL2/SDL.h>
@@ -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;
 }