From 96226b5a7799bf98532742e20db595dfb0662d2e Mon Sep 17 00:00:00 2001 From: "Christian W. Zuckschwerdt" Date: Sat, 21 Oct 2023 19:55:41 +0200 Subject: [PATCH] minor: Add GCC Static analysis (closes #1372) --- CMakeLists.txt | 6 ++++++ src/data.c | 7 ++++++- src/decoder_util.c | 13 +++++++------ src/http_server.c | 2 +- src/optparse.c | 3 +++ src/output_file.c | 8 ++++---- src/output_influx.c | 5 ++++- src/output_log.c | 2 +- src/output_mqtt.c | 2 +- src/output_rtltcp.c | 20 ++++++++++++++++---- src/output_trigger.c | 2 +- src/output_udp.c | 2 +- src/pulse_data.c | 13 +++++++++++++ src/r_util.c | 6 ++++++ src/sdr.c | 13 ++++++++++++- src/write_sigrok.c | 12 +++++++++--- tests/baseband-test.c | 25 +++++++++++++++++++++++++ 17 files changed, 116 insertions(+), 25 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index d1ce058e2..20311e07a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -100,6 +100,12 @@ if("${CMAKE_C_COMPILER_ID}" MATCHES "Clang") ADD_DEFINITIONS(-Wlarge-by-value-copy=8) endif() +# Enable Static analysis on GCC10+ +if("${CMAKE_C_COMPILER_ID}" STREQUAL "GNU" AND CMAKE_C_COMPILER_VERSION VERSION_GREATER_EQUAL "10.0.0") + message(STATUS "Using GCC Static analysis") + add_definitions(-fanalyzer) +endif() + # Shut MSVC up about strdup and strtok if(MSVC) ADD_DEFINITIONS(-D_CRT_NONSTDC_NO_DEPRECATE) diff --git a/src/data.c b/src/data.c index c13354350..3a5ed2573 100644 --- a/src/data.c +++ b/src/data.c @@ -151,6 +151,10 @@ R_API data_array_t *data_array(int num_values, data_type_t type, void const *val return NULL; } +// the static analyzer can't prove the allocs to be correct +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wanalyzer-malloc-leak" + static data_t *vdata_make(data_t *first, const char *key, const char *pretty_key, va_list ap) { data_type_t type; @@ -257,7 +261,6 @@ static data_t *vdata_make(data_t *first, const char *key, const char *pretty_key type = va_arg(ap, data_type_t); } } while (key); - va_end(ap); if (format) { fprintf(stderr, "vdata_make() format type without data\n"); goto alloc_error; @@ -344,6 +347,8 @@ R_API void data_free(data_t *data) } } +#pragma GCC diagnostic pop + /* data output */ R_API void data_output_print(data_output_t *output, data_t *data) diff --git a/src/decoder_util.c b/src/decoder_util.c index fff89fa34..337fb3373 100644 --- a/src/decoder_util.c +++ b/src/decoder_util.c @@ -137,10 +137,11 @@ void decoder_log_bitbuffer(r_device *decoder, int level, char const *func, const // note that decoder levels start at LOG_WARNING level += 4; - char *row_codes[BITBUF_ROWS]; + char *row_codes[BITBUF_ROWS] = {0}; char *row_bits[BITBUF_ROWS] = {0}; - for (unsigned i = 0; i < bitbuffer->num_rows; i++) { + unsigned num_rows = bitbuffer->num_rows; + for (unsigned i = 0; i < num_rows; i++) { row_codes[i] = bitrow_asprint_code(bitbuffer->bb[i], bitbuffer->bits_per_row[i]); if (decoder->verbose_bits) { @@ -153,20 +154,20 @@ void decoder_log_bitbuffer(r_device *decoder, int level, char const *func, const "src", "", DATA_STRING, func, "lvl", "", DATA_INT, level, "msg", "", DATA_STRING, msg, - "num_rows", "", DATA_INT, bitbuffer->num_rows, - "codes", "", DATA_ARRAY, data_array(bitbuffer->num_rows, DATA_STRING, row_codes), + "num_rows", "", DATA_INT, num_rows, + "codes", "", DATA_ARRAY, data_array(num_rows, DATA_STRING, row_codes), NULL); /* clang-format on */ if (decoder->verbose_bits) { data_append(data, - "bits", "", DATA_ARRAY, data_array(bitbuffer->num_rows, DATA_STRING, row_bits), + "bits", "", DATA_ARRAY, data_array(num_rows, DATA_STRING, row_bits), NULL); } decoder_output_log(decoder, level, data); - for (unsigned i = 0; i < bitbuffer->num_rows; i++) { + for (unsigned i = 0; i < num_rows; i++) { free(row_codes[i]); free(row_bits[i]); } diff --git a/src/http_server.c b/src/http_server.c index 4ecc3370f..6fee232b7 100644 --- a/src/http_server.c +++ b/src/http_server.c @@ -1253,5 +1253,5 @@ struct data_output *data_output_http_create(struct mg_mgr *mgr, char const *host exit(1); } - return &http->output; + return (struct data_output *)http; } diff --git a/src/optparse.c b/src/optparse.c index 58ff9359c..e4e0e1d37 100644 --- a/src/optparse.c +++ b/src/optparse.c @@ -330,6 +330,9 @@ char *asepcb(char **stringp, char delim, char stop) int kwargs_match(char const *s, char const *key, char const **val) { + if (!key || !*key) { + return 0; // no match + } size_t len = strlen(key); // check prefix match if (strncmp(s, key, len)) { diff --git a/src/output_file.c b/src/output_file.c index c7808ded4..0f342cb16 100644 --- a/src/output_file.c +++ b/src/output_file.c @@ -148,7 +148,7 @@ struct data_output *data_output_json_create(int log_level, FILE *file) json->output.output_free = data_output_json_free; json->file = file; - return &json->output; + return (struct data_output *)json; } /* Pretty Key-Value printer */ @@ -424,7 +424,7 @@ struct data_output *data_output_kv_create(int log_level, FILE *file) kv->ring_bell = 0; // TODO: enable if requested... - return &kv->output; + return (struct data_output *)kv; } /* CSV printer */ @@ -469,7 +469,7 @@ static void R_API_CALLCONV print_csv_string(data_output_t *output, const char *s UNUSED(format); data_output_csv_t *csv = (data_output_csv_t *)output; - while (*str) { + while (str && *str) { if (strncmp(str, csv->separator, strlen(csv->separator)) == 0) fputc('\\', csv->file); fputc(*str, csv->file); @@ -638,5 +638,5 @@ struct data_output *data_output_csv_create(int log_level, FILE *file) csv->output.output_free = data_output_csv_free; csv->file = file; - return &csv->output; + return (struct data_output *)csv; } diff --git a/src/output_influx.c b/src/output_influx.c index e0782aff6..e1b0609c7 100644 --- a/src/output_influx.c +++ b/src/output_influx.c @@ -458,6 +458,9 @@ struct data_output *data_output_influx_create(struct mg_mgr *mgr, char *opts) char *token = NULL; // param/opts starts with URL + if (!opts) { + opts = ""; + } char *url = opts; opts = strchr(opts, ','); if (opts) { @@ -515,5 +518,5 @@ struct data_output *data_output_influx_create(struct mg_mgr *mgr, char *opts) influx->mgr = mgr; influx_client_init(influx, url, token); - return &influx->output; + return (struct data_output *)influx; } diff --git a/src/output_log.c b/src/output_log.c index e5e2df668..0973e2a89 100644 --- a/src/output_log.c +++ b/src/output_log.c @@ -164,5 +164,5 @@ struct data_output *data_output_log_create(int log_level, FILE *file) log->output.output_free = data_output_log_free; log->file = file; - return &log->output; + return (struct data_output *)log; } diff --git a/src/output_mqtt.c b/src/output_mqtt.c index 4d60f332e..23015f5f4 100644 --- a/src/output_mqtt.c +++ b/src/output_mqtt.c @@ -599,5 +599,5 @@ struct data_output *data_output_mqtt_create(struct mg_mgr *mgr, char *param, cha mqtt->mqc = mqtt_client_init(mgr, &tls_opts, host, port, user, pass, client_id, retain, qos); - return &mqtt->output; + return (struct data_output *)mqtt; } diff --git a/src/output_rtltcp.c b/src/output_rtltcp.c index f30438239..2ec4a3738 100644 --- a/src/output_rtltcp.c +++ b/src/output_rtltcp.c @@ -254,14 +254,21 @@ static THREAD_RETURN THREAD_CALL accept_thread(void *arg) rtltcp_server_t *srv = arg; // Start listening for clients, waits for an incoming connection - listen(srv->sock, 1); + int listen_sock = srv->sock; // make it easy for the checker + int r = listen(listen_sock, 1); + if (r < 0) { + perror("ERROR on listen"); + closesocket(listen_sock); + srv->sock = INVALID_SOCKET; + return 0; + } // print_log(LOG_DEBUG, "rtl_tcp", "rtl_tcp listening..."); for (;;) { // Accept actual connection from the client struct sockaddr_storage addr = {0}; unsigned addr_len = sizeof(addr); - int sock = accept(srv->sock, (struct sockaddr *)&addr, &addr_len); + int sock = accept(listen_sock, (struct sockaddr *)&addr, &addr_len); // TODO: ignore ECONNABORTED (Software caused connection abort) if (sock < 0) { @@ -274,6 +281,7 @@ static THREAD_RETURN THREAD_CALL accept_thread(void *arg) int opt = 1; if (setsockopt(sock, SOL_SOCKET, SO_NOSIGPIPE, &opt, sizeof(opt)) == -1) { perror("setsockopt"); + closesocket(sock); continue; } #endif @@ -285,6 +293,7 @@ static THREAD_RETURN THREAD_CALL accept_thread(void *arg) host, sizeof(host), port, sizeof(port), NI_NUMERICHOST | NI_NUMERICSERV); if (err != 0) { print_logf(LOG_ERROR, __func__, "failed to convert address to string (code=%d)", err); + closesocket(sock); continue; } print_logf(LOG_NOTICE, "rtl_tcp", "client connected from %s port %s", host, port); @@ -388,7 +397,6 @@ static int rtltcp_server_start(rtltcp_server_t *srv, char const *host, char cons for (res = res0; res; res = res->ai_next) { sock = socket(res->ai_family, res->ai_socktype, res->ai_protocol); if (sock >= 0) { - srv->sock = sock; memset(&srv->addr, 0, sizeof(srv->addr)); memcpy(&srv->addr, res->ai_addr, res->ai_addrlen); srv->addr_len = res->ai_addrlen; @@ -403,9 +411,11 @@ static int rtltcp_server_start(rtltcp_server_t *srv, char const *host, char cons if (bind(sock, (struct sockaddr *)&srv->addr, srv->addr_len) < 0) { perror("error on binding"); + closesocket(sock); return -1; } + srv->sock = sock; srv->cfg = cfg; srv->output = output; @@ -416,6 +426,7 @@ static int rtltcp_server_start(rtltcp_server_t *srv, char const *host, char cons address, sizeof(address), portstr, sizeof(portstr), NI_NUMERICHOST | NI_NUMERICSERV); if (err != 0) { print_logf(LOG_ERROR, __func__, "failed to convert address to string (code=%d)", err); + closesocket(sock); return -1; } print_logf(LOG_CRITICAL, "rtl_tcp server", "Serving rtl_tcp on address %s %s", address, portstr); @@ -436,6 +447,7 @@ static int rtltcp_server_start(rtltcp_server_t *srv, char const *host, char cons #endif if (r) { fprintf(stderr, "%s: error in pthread_create, rc: %d\n", __func__, r); + closesocket(sock); } return r; @@ -531,7 +543,7 @@ struct raw_output *raw_output_rtltcp_create(const char *host, const char *port, exit(1); } - return &rtltcp->output; + return (struct raw_output *)rtltcp; } #else diff --git a/src/output_trigger.c b/src/output_trigger.c index e668d1ca1..1e351b916 100644 --- a/src/output_trigger.c +++ b/src/output_trigger.c @@ -54,5 +54,5 @@ struct data_output *data_output_trigger_create(FILE *file) trigger->output.output_free = data_output_trigger_free; trigger->file = file; - return &trigger->output; + return (struct data_output *)trigger; } diff --git a/src/output_udp.c b/src/output_udp.c index 34f7ed78a..9156322ff 100644 --- a/src/output_udp.c +++ b/src/output_udp.c @@ -235,5 +235,5 @@ struct data_output *data_output_syslog_create(int log_level, const char *host, c syslog->hostname[_POSIX_HOST_NAME_MAX] = '\0'; datagram_client_open(&syslog->client, host, port); - return &syslog->output; + return (struct data_output *)syslog; } diff --git a/src/pulse_data.c b/src/pulse_data.c index f65cde2e0..2531afd10 100644 --- a/src/pulse_data.c +++ b/src/pulse_data.c @@ -13,6 +13,7 @@ #include "pulse_data.h" #include "rfraw.h" #include "r_util.h" +#include "fatal.h" #include #include #include @@ -74,6 +75,10 @@ static inline void chk_ret(int ret) void pulse_data_print_vcd_header(FILE *file, uint32_t sample_rate) { + if (!file) { + FATAL("Invalid stream in pulse_data_print_vcd_header()"); + } + char time_str[LOCAL_TIME_BUFLEN]; char *timescale; if (sample_rate <= 500000) @@ -161,6 +166,10 @@ void pulse_data_load(FILE *file, pulse_data_t *data, uint32_t sample_rate) void pulse_data_print_pulse_header(FILE *file) { + if (!file) { + FATAL("Invalid stream in pulse_data_print_pulse_header()"); + } + char time_str[LOCAL_TIME_BUFLEN]; chk_ret(fprintf(file, ";pulse data\n")); @@ -172,6 +181,10 @@ void pulse_data_print_pulse_header(FILE *file) void pulse_data_dump(FILE *file, pulse_data_t const *data) { + if (!file) { + FATAL("Invalid stream in pulse_data_dump()"); + } + char time_str[LOCAL_TIME_BUFLEN]; chk_ret(fprintf(file, ";received %s\n", format_time_str(time_str, NULL, 1, 0))); diff --git a/src/r_util.c b/src/r_util.c index 85eb32201..0102ba95d 100644 --- a/src/r_util.c +++ b/src/r_util.c @@ -146,6 +146,12 @@ float inhg2hpa(float inhg) bool str_endswith(char const *restrict str, char const *restrict suffix) { + if (!suffix) { + return true; + } + if (!str) { + return false; + } int str_len = strlen(str); int suffix_len = strlen(suffix); diff --git a/src/sdr.c b/src/sdr.c index c09ba45c3..70284ea73 100644 --- a/src/sdr.c +++ b/src/sdr.c @@ -177,6 +177,7 @@ static int rtltcp_open(sdr_dev_t **out_dev, char const *dev_query, int verbose) ret = connect(sock, res->ai_addr, res->ai_addrlen); if (ret == -1) { perror("connect"); + closesocket(sock); sock = INVALID_SOCKET; } else @@ -954,6 +955,10 @@ static int sdr_open_soapy(sdr_dev_t **out_dev, char const *dev_query, int verbos return 0; } +// the buffer sizes can't be proven to be correct +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wanalyzer-allocation-size" + static int soapysdr_read_loop(sdr_dev_t *dev, sdr_event_cb_t cb, void *ctx, uint32_t buf_num, uint32_t buf_len) { size_t buffer_size = (size_t)buf_num * buf_len; @@ -1050,6 +1055,8 @@ static int soapysdr_read_loop(sdr_dev_t *dev, sdr_event_cb_t cb, void *ctx, uint return 0; } +#pragma GCC diagnostic pop + #endif /* Public API */ @@ -1509,8 +1516,12 @@ int sdr_apply_settings(sdr_dev_t *dev, char const *sdr_settings, int verbose) for (size_t i = 0; i < settings.size; ++i) { const char *key = settings.keys[i]; const char *value = settings.vals[i]; - if (verbose) + if (!key) { + continue; + } + if (verbose) { print_logf(LOG_NOTICE, "SDR", "Setting %s to %s", key, value); + } if (!strcmp(key, "antenna")) { if (SoapySDRDevice_setAntenna(dev->soapy_dev, SOAPY_SDR_RX, 0, value) != 0) { r = -1; diff --git a/src/write_sigrok.c b/src/write_sigrok.c index 7b40625e5..e752b2459 100644 --- a/src/write_sigrok.c +++ b/src/write_sigrok.c @@ -158,15 +158,17 @@ void write_sigrok(char const *filename, unsigned samplerate, unsigned probes, un argv[arg++] = "logic-1-1"; } + char *argv_dups[30] = {0}; // store only dups to help the checker match the free() char **argv_analog = &argv[arg]; char str_buf[64]; for (unsigned i = probes + 1; i <= probes + analogs; ++i) { snprintf(str_buf, sizeof(str_buf), "analog-1-%u-1", i); char* dup = strdup(str_buf); if (!dup) { - FATAL_STRDUP("write_sigrok()"); + FATAL_STRDUP("write_sigrok()"); } argv[arg++] = dup; + argv_dups[arg++] = dup; } int status = 0; @@ -178,8 +180,9 @@ void write_sigrok(char const *filename, unsigned samplerate, unsigned probes, un // child process because return value zero execvp(argv[0], argv); // execvp() returns only on error - for (int i = 0; i < arg; ++i) + for (int i = 0; i < arg; ++i) { fprintf(stderr, "%s ", argv[i]); + } fprintf(stderr, "\n"); perror("execvp"); exit(1); @@ -211,8 +214,11 @@ void write_sigrok(char const *filename, unsigned samplerate, unsigned probes, un if (unlink(argv_analog[i])) { perror("unlinking Sigrok \"analog-1-N-1\" file"); } - free(argv_analog[i]); } + for (int i = 0; i < arg; ++i) { + free(argv_dups[i]); + } + #endif // !_WIN32 } diff --git a/tests/baseband-test.c b/tests/baseband-test.c index 55806cf57..20ac93927 100644 --- a/tests/baseband-test.c +++ b/tests/baseband-test.c @@ -34,6 +34,7 @@ typedef SSIZE_T ssize_t; #include +#include "fatal.h" #include "baseband.h" #define MEASURE(label, block) \ @@ -94,6 +95,9 @@ int main(int argc, char *argv[]) filename = argv[1]; cu8_buf = malloc(sizeof(uint8_t) * 2 * max_block_size); + if (!cu8_buf) { + FATAL_MALLOC("main()"); + } n_read = read_buf(filename, cu8_buf, sizeof(uint8_t) * 2 * max_block_size); if (n_read < 1) { free(cu8_buf); @@ -101,12 +105,33 @@ int main(int argc, char *argv[]) } y16_buf = malloc(sizeof(uint16_t) * max_block_size); + if (!y16_buf) { + FATAL_MALLOC("main()"); + } cs16_buf = malloc(sizeof(int16_t) * 2 * max_block_size); + if (!cs16_buf) { + FATAL_MALLOC("main()"); + } y32_buf = malloc(sizeof(uint32_t) * max_block_size); + if (!y32_buf) { + FATAL_MALLOC("main()"); + } u16_buf = malloc(sizeof(uint16_t) * max_block_size); + if (!u16_buf) { + FATAL_MALLOC("main()"); + } u32_buf = malloc(sizeof(uint32_t) * max_block_size); + if (!u32_buf) { + FATAL_MALLOC("main()"); + } s16_buf = malloc(sizeof(int16_t) * max_block_size); + if (!s16_buf) { + FATAL_MALLOC("main()"); + } s32_buf = malloc(sizeof(int32_t) * max_block_size); + if (!s32_buf) { + FATAL_MALLOC("main()"); + } n_samples = n_read / (sizeof(uint8_t) * 2);