From 4b8cb042c41e29002d562e80809a6b3a9f01e49f Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Fri, 18 Feb 2022 21:16:53 +0100 Subject: [PATCH] Use vector for listing ADB devices This avoids the hardcoded maximum number of ADB devices detected (16). Refs #3029 PR #3035 Co-authored-by: Daniel Ansorregui --- app/src/adb/adb.c | 44 +++++++++---------- app/src/adb/adb_device.c | 7 +-- app/src/adb/adb_device.h | 6 ++- app/src/adb/adb_parser.c | 30 ++++++------- app/src/adb/adb_parser.h | 5 +-- app/tests/test_adb_parser.c | 88 +++++++++++++++++++++---------------- 6 files changed, 95 insertions(+), 85 deletions(-) diff --git a/app/src/adb/adb.c b/app/src/adb/adb.c index 4ddb93f3..c14ded92 100644 --- a/app/src/adb/adb.c +++ b/app/src/adb/adb.c @@ -5,6 +5,7 @@ #include #include +#include "adb_device.h" #include "adb_parser.h" #include "util/file.h" #include "util/log.h" @@ -392,16 +393,16 @@ sc_adb_disconnect(struct sc_intr *intr, const char *ip_port, unsigned flags) { return process_check_success_intr(intr, pid, "adb disconnect", flags); } -static ssize_t +static bool sc_adb_list_devices(struct sc_intr *intr, unsigned flags, - struct sc_adb_device *devices, size_t len) { + struct sc_vec_adb_devices *out_vec) { const char *const argv[] = SC_ADB_COMMAND("devices", "-l"); sc_pipe pout; sc_pid pid = sc_adb_execute_p(argv, flags, &pout); if (pid == SC_PROCESS_NONE) { LOGE("Could not execute \"adb devices -l\""); - return -1; + return false; } char buf[4096]; @@ -410,11 +411,11 @@ sc_adb_list_devices(struct sc_intr *intr, unsigned flags, bool ok = process_check_success_intr(intr, pid, "adb devices -l", flags); if (!ok) { - return -1; + return false; } if (r == -1) { - return -1; + return false; } assert((size_t) r < sizeof(buf)); @@ -423,14 +424,14 @@ sc_adb_list_devices(struct sc_intr *intr, unsigned flags, // in the buffer in a single pass LOGW("Result of \"adb devices -l\" does not fit in 4Kb. " "Please report an issue."); - return -1; + return false; } // It is parsed as a NUL-terminated string buf[r] = '\0'; // List all devices to the output list directly - return sc_adb_parse_devices(buf, devices, len); + return sc_adb_parse_devices(buf, out_vec); } static bool @@ -529,22 +530,21 @@ bool sc_adb_select_device(struct sc_intr *intr, const struct sc_adb_device_selector *selector, unsigned flags, struct sc_adb_device *out_device) { - struct sc_adb_device devices[16]; - ssize_t count = - sc_adb_list_devices(intr, flags, devices, ARRAY_LEN(devices)); - if (count == -1) { + struct sc_vec_adb_devices vec = SC_VECTOR_INITIALIZER; + bool ok = sc_adb_list_devices(intr, flags, &vec); + if (!ok) { LOGE("Could not list ADB devices"); return false; } - if (count == 0) { + if (vec.size == 0) { LOGE("Could not find any ADB device"); return false; } size_t sel_idx; // index of the single matching device if sel_count == 1 size_t sel_count = - sc_adb_devices_select(devices, count, selector, &sel_idx); + sc_adb_devices_select(vec.data, vec.size, selector, &sel_idx); if (sel_count == 0) { // if count > 0 && sel_count == 0, then necessarily a selection is @@ -567,8 +567,8 @@ sc_adb_select_device(struct sc_intr *intr, break; } - sc_adb_devices_log(SC_LOG_LEVEL_ERROR, devices, count); - sc_adb_devices_destroy_all(devices, count); + sc_adb_devices_log(SC_LOG_LEVEL_ERROR, vec.data, vec.size); + sc_adb_devices_destroy(&vec); return false; } @@ -594,28 +594,28 @@ sc_adb_select_device(struct sc_intr *intr, assert(!"Unexpected selector type"); break; } - sc_adb_devices_log(SC_LOG_LEVEL_ERROR, devices, count); + sc_adb_devices_log(SC_LOG_LEVEL_ERROR, vec.data, vec.size); LOGE("Select a device via -s (--serial), -d (--select-usb) or -e " "(--select-tcpip)"); - sc_adb_devices_destroy_all(devices, count); + sc_adb_devices_destroy(&vec); return false; } assert(sel_count == 1); // sel_idx is valid only if sel_count == 1 - struct sc_adb_device *device = &devices[sel_idx]; + struct sc_adb_device *device = &vec.data[sel_idx]; - bool ok = sc_adb_device_check_state(device, devices, count); + ok = sc_adb_device_check_state(device, vec.data, vec.size); if (!ok) { - sc_adb_devices_destroy_all(devices, count); + sc_adb_devices_destroy(&vec); return false; } LOGD("ADB device found:"); - sc_adb_devices_log(SC_LOG_LEVEL_DEBUG, devices, count); + sc_adb_devices_log(SC_LOG_LEVEL_DEBUG, vec.data, vec.size); // Move devics into out_device (do not destroy device) sc_adb_device_move(out_device, device); - sc_adb_devices_destroy_all(devices, count); + sc_adb_devices_destroy(&vec); return true; } diff --git a/app/src/adb/adb_device.c b/app/src/adb/adb_device.c index b6ff16a7..cfd5dc30 100644 --- a/app/src/adb/adb_device.c +++ b/app/src/adb/adb_device.c @@ -18,9 +18,10 @@ sc_adb_device_move(struct sc_adb_device *dst, struct sc_adb_device *src) { } void -sc_adb_devices_destroy_all(struct sc_adb_device *devices, size_t count) { - for (size_t i = 0; i < count; ++i) { - sc_adb_device_destroy(&devices[i]); +sc_adb_devices_destroy(struct sc_vec_adb_devices *devices) { + for (size_t i = 0; i < devices->size; ++i) { + sc_adb_device_destroy(&devices->data[i]); } + sc_vector_destroy(devices); } diff --git a/app/src/adb/adb_device.h b/app/src/adb/adb_device.h index ed8362e9..14d0408c 100644 --- a/app/src/adb/adb_device.h +++ b/app/src/adb/adb_device.h @@ -6,6 +6,8 @@ #include #include +#include "util/vector.h" + struct sc_adb_device { char *serial; char *state; @@ -13,6 +15,8 @@ struct sc_adb_device { bool selected; }; +struct sc_vec_adb_devices SC_VECTOR(struct sc_adb_device); + void sc_adb_device_destroy(struct sc_adb_device *device); @@ -29,6 +33,6 @@ void sc_adb_device_move(struct sc_adb_device *dst, struct sc_adb_device *src); void -sc_adb_devices_destroy_all(struct sc_adb_device *devices, size_t count); +sc_adb_devices_destroy(struct sc_vec_adb_devices *devices); #endif diff --git a/app/src/adb/adb_parser.c b/app/src/adb/adb_parser.c index 85e8ffaf..933eafbb 100644 --- a/app/src/adb/adb_parser.c +++ b/app/src/adb/adb_parser.c @@ -109,11 +109,8 @@ sc_adb_parse_device(char *line, struct sc_adb_device *device) { return true; } -ssize_t -sc_adb_parse_devices(char *str, struct sc_adb_device *devices, - size_t devices_len) { - size_t dev_count = 0; - +bool +sc_adb_parse_devices(char *str, struct sc_vec_adb_devices *out_vec) { #define HEADER "List of devices attached" #define HEADER_LEN (sizeof(HEADER) - 1) bool header_found = false; @@ -144,25 +141,24 @@ sc_adb_parse_devices(char *str, struct sc_adb_device *devices, size_t line_len = sc_str_remove_trailing_cr(line, len); line[line_len] = '\0'; - bool ok = sc_adb_parse_device(line, &devices[dev_count]); + struct sc_adb_device device; + bool ok = sc_adb_parse_device(line, &device); if (!ok) { continue; } - ++dev_count; - - assert(dev_count <= devices_len); - if (dev_count == devices_len) { - // Max number of devices reached - break; + ok = sc_vector_push(out_vec, device); + if (!ok) { + LOG_OOM(); + LOGE("Could not push adb_device to vector"); + sc_adb_device_destroy(&device); + // continue anyway + continue; } } - if (!header_found) { - return -1; - } - - return dev_count; + assert(header_found || out_vec->size == 0); + return header_found; } static char * diff --git a/app/src/adb/adb_parser.h b/app/src/adb/adb_parser.h index 65493a2e..e0cc389b 100644 --- a/app/src/adb/adb_parser.h +++ b/app/src/adb/adb_parser.h @@ -14,9 +14,8 @@ * * Warning: this function modifies the buffer for optimization purposes. */ -ssize_t -sc_adb_parse_devices(char *str, struct sc_adb_device *devices, - size_t devices_len); +bool +sc_adb_parse_devices(char *str, struct sc_vec_adb_devices *out_vec); /** * Parse the ip from the output of `adb shell ip route` diff --git a/app/tests/test_adb_parser.c b/app/tests/test_adb_parser.c index c4b18a8d..1a127632 100644 --- a/app/tests/test_adb_parser.c +++ b/app/tests/test_adb_parser.c @@ -13,21 +13,22 @@ static void test_adb_devices() { "192.168.1.1:5555 device product:MyWifiProduct model:MyWifiModel " "device:MyWifiDevice trandport_id:2\n"; - struct sc_adb_device devices[16]; - ssize_t count = sc_adb_parse_devices(output, devices, ARRAY_LEN(devices)); - assert(count == 2); + struct sc_vec_adb_devices vec = SC_VECTOR_INITIALIZER; + bool ok = sc_adb_parse_devices(output, &vec); + assert(ok); + assert(vec.size == 2); - struct sc_adb_device *device = &devices[0]; + struct sc_adb_device *device = &vec.data[0]; assert(!strcmp("0123456789abcdef", device->serial)); assert(!strcmp("device", device->state)); assert(!strcmp("MyModel", device->model)); - device = &devices[1]; + device = &vec.data[1]; assert(!strcmp("192.168.1.1:5555", device->serial)); assert(!strcmp("device", device->state)); assert(!strcmp("MyWifiModel", device->model)); - sc_adb_devices_destroy_all(devices, count); + sc_adb_devices_destroy(&vec); } static void test_adb_devices_cr() { @@ -38,21 +39,22 @@ static void test_adb_devices_cr() { "192.168.1.1:5555 device product:MyWifiProduct model:MyWifiModel " "device:MyWifiDevice trandport_id:2\r\n"; - struct sc_adb_device devices[16]; - ssize_t count = sc_adb_parse_devices(output, devices, ARRAY_LEN(devices)); - assert(count == 2); + struct sc_vec_adb_devices vec = SC_VECTOR_INITIALIZER; + bool ok = sc_adb_parse_devices(output, &vec); + assert(ok); + assert(vec.size == 2); - struct sc_adb_device *device = &devices[0]; + struct sc_adb_device *device = &vec.data[0]; assert(!strcmp("0123456789abcdef", device->serial)); assert(!strcmp("device", device->state)); assert(!strcmp("MyModel", device->model)); - device = &devices[1]; + device = &vec.data[1]; assert(!strcmp("192.168.1.1:5555", device->serial)); assert(!strcmp("device", device->state)); assert(!strcmp("MyWifiModel", device->model)); - sc_adb_devices_destroy_all(devices, count); + sc_adb_devices_destroy(&vec); } static void test_adb_devices_daemon_start() { @@ -63,16 +65,17 @@ static void test_adb_devices_daemon_start() { "0123456789abcdef device usb:2-1 product:MyProduct model:MyModel " "device:MyDevice transport_id:1\n"; - struct sc_adb_device devices[16]; - ssize_t count = sc_adb_parse_devices(output, devices, ARRAY_LEN(devices)); - assert(count == 1); + struct sc_vec_adb_devices vec = SC_VECTOR_INITIALIZER; + bool ok = sc_adb_parse_devices(output, &vec); + assert(ok); + assert(vec.size == 1); - struct sc_adb_device *device = &devices[0]; + struct sc_adb_device *device = &vec.data[0]; assert(!strcmp("0123456789abcdef", device->serial)); assert(!strcmp("device", device->state)); assert(!strcmp("MyModel", device->model)); - sc_adb_device_destroy(device); + sc_adb_devices_destroy(&vec); } static void test_adb_devices_daemon_start_mixed() { @@ -84,21 +87,22 @@ static void test_adb_devices_daemon_start_mixed() { "87654321 device usb:2-1 product:MyProduct model:MyModel " "device:MyDevice\n"; - struct sc_adb_device devices[16]; - ssize_t count = sc_adb_parse_devices(output, devices, ARRAY_LEN(devices)); - assert(count == 2); + struct sc_vec_adb_devices vec = SC_VECTOR_INITIALIZER; + bool ok = sc_adb_parse_devices(output, &vec); + assert(ok); + assert(vec.size == 2); - struct sc_adb_device *device = &devices[0]; + struct sc_adb_device *device = &vec.data[0]; assert(!strcmp("0123456789abcdef", device->serial)); assert(!strcmp("unauthorized", device->state)); assert(!device->model); - device = &devices[1]; + device = &vec.data[1]; assert(!strcmp("87654321", device->serial)); assert(!strcmp("device", device->state)); assert(!strcmp("MyModel", device->model)); - sc_adb_devices_destroy_all(devices, count); + sc_adb_devices_destroy(&vec); } static void test_adb_devices_without_eol() { @@ -106,34 +110,39 @@ static void test_adb_devices_without_eol() { "List of devices attached\n" "0123456789abcdef device usb:2-1 product:MyProduct model:MyModel " "device:MyDevice transport_id:1"; - struct sc_adb_device devices[16]; - ssize_t count = sc_adb_parse_devices(output, devices, ARRAY_LEN(devices)); - assert(count == 1); - struct sc_adb_device *device = &devices[0]; + struct sc_vec_adb_devices vec = SC_VECTOR_INITIALIZER; + bool ok = sc_adb_parse_devices(output, &vec); + assert(ok); + assert(vec.size == 1); + + struct sc_adb_device *device = &vec.data[0]; assert(!strcmp("0123456789abcdef", device->serial)); assert(!strcmp("device", device->state)); assert(!strcmp("MyModel", device->model)); - sc_adb_device_destroy(device); + sc_adb_devices_destroy(&vec); } static void test_adb_devices_without_header() { char output[] = "0123456789abcdef device usb:2-1 product:MyProduct model:MyModel " "device:MyDevice transport_id:1\n"; - struct sc_adb_device devices[16]; - ssize_t count = sc_adb_parse_devices(output, devices, ARRAY_LEN(devices)); - assert(count == -1); + + struct sc_vec_adb_devices vec = SC_VECTOR_INITIALIZER; + bool ok = sc_adb_parse_devices(output, &vec); + assert(!ok); } static void test_adb_devices_corrupted() { char output[] = "List of devices attached\n" "corrupted_garbage\n"; - struct sc_adb_device devices[16]; - ssize_t count = sc_adb_parse_devices(output, devices, ARRAY_LEN(devices)); - assert(count == 0); + + struct sc_vec_adb_devices vec = SC_VECTOR_INITIALIZER; + bool ok = sc_adb_parse_devices(output, &vec); + assert(ok); + assert(vec.size == 0); } static void test_adb_devices_spaces() { @@ -141,16 +150,17 @@ static void test_adb_devices_spaces() { "List of devices attached\n" "0123456789abcdef unauthorized usb:1-4 transport_id:3\n"; - struct sc_adb_device devices[16]; - ssize_t count = sc_adb_parse_devices(output, devices, ARRAY_LEN(devices)); - assert(count == 1); + struct sc_vec_adb_devices vec = SC_VECTOR_INITIALIZER; + bool ok = sc_adb_parse_devices(output, &vec); + assert(ok); + assert(vec.size == 1); - struct sc_adb_device *device = &devices[0]; + struct sc_adb_device *device = &vec.data[0]; assert(!strcmp("0123456789abcdef", device->serial)); assert(!strcmp("unauthorized", device->state)); assert(!device->model); - sc_adb_device_destroy(device); + sc_adb_devices_destroy(&vec); } static void test_get_ip_single_line() {