minor: Fix flex device memory leak during destruction (#3350)

.github/workflows/check.yml

@@ -1,3 +1,5 @@
+# https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax
+# https://docs.github.com/en/actions/reference/runners/github-hosted-runners
 name: Build Analyze Check
 on: [push, pull_request]
 jobs:
@@ -15,26 +17,48 @@ jobs:
       - name: Configure
         run: cmake -B build
       - name: Build
-        run: cmake --build build
+        run: cmake --build build -- -j $(nproc)
+      - name: Test
+        run: |
+          cmake --build build --target test || \
+          { cat build/Testing/Temporary/LastTest.log && false; }
 
   build_check_job:
     runs-on: ubuntu-latest
-    name: Build with CMake
+    strategy:
+      fail-fast: false
+      matrix:
+        cmake-generator: [Unix Makefiles, Ninja]
+        cmake-build-type: [Debug, Release]
+    name: Build ${{ matrix.cmake-build-type }} ${{ matrix.cmake-generator }}
     steps:
       - uses: actions/checkout@v5
-      - name: Setup
+      - name: Apt Update
+        run: sudo apt-get update -q -y --no-install-recommends
+      - name: Install Build Tools
+        run: sudo apt-get install -q -y --no-install-recommends cmake ninja-build
+      - name: Install Libraries
+        run: sudo apt-get install -q -y --no-install-recommends libsoapysdr-dev librtlsdr-dev
+      - name: Versions
         run: |
-          sudo apt-get update -q -y
-          sudo apt-get install -q -y --no-install-recommends cmake ninja-build
-          sudo apt-get install -q -y libsoapysdr-dev librtlsdr-dev
-      - name: Configure CMake+Ninja
-        run: cmake -GNinja -B ${{ runner.workspace }}/b/ninja -DENABLE_RTLSDR=OFF -DENABLE_SOAPYSDR=OFF
-      - name: Build CMake+Ninja
-        run: cmake --build ${{ runner.workspace }}/b/ninja
-      - name: Configure CMake+UnixMakefiles
-        run: cmake -G"Unix Makefiles" -B ${{ runner.workspace }}/b/unixmakefiles -DENABLE_RTLSDR=OFF -DENABLE_SOAPYSDR=OFF
-      - name: Build CMake+UnixMakefiles
-        run: cmake --build ${{ runner.workspace }}/b/unixmakefiles
+          cmake --version
+          lsb_release -a
+          make --version
+          echo Ninja Version: $(ninja --version)
+          uname -a
+      - name: Configure
+        run: |
+          cmake \
+            -B build \
+            -DCMAKE_BUILD_TYPE=${{ matrix.cmake-build-type }} \
+            -DENABLE_RTLSDR=OFF -DENABLE_SOAPYSDR=OFF \
+            -G "${{ matrix.cmake-generator }}"
+      - name: Build
+        run: cmake --build build
+      - name: Test
+        run: |
+          cmake --build build --target test || \
+          { cat build/Testing/Temporary/LastTest.log && false; }
 
   doc_check_job:
     runs-on: ubuntu-latest

.gitignore

@@ -29,3 +29,6 @@ cmake-build-debug/
 # File manager files
 .DS_Store
 .history
+
+# Precompiled headers
+*.h.pch

include/r_device.h

@@ -68,7 +68,8 @@ typedef struct r_device {
     float sync_width;
     float tolerance;
     int (*decode_fn)(struct r_device *decoder, struct bitbuffer *bitbuffer);
-    struct r_device *(*create_fn)(char *args);
+    struct r_device *(*create_fn)(char const *args);
+    void (*destroy_fn)(struct r_device *r_dev);
     unsigned priority; ///< Run later and only if no previous events were produced
     unsigned disabled; ///< 0: default enabled, 1: default disabled, 2: disabled, 3: disabled and hidden
     char const *const *fields; ///< List of fields this decoder produces; required for CSV output. NULL-terminated.

include/rtl_433_devices.h

@@ -329,7 +329,8 @@
     DECL(shenzhen_wale_wl_th6r) \
     /* Add new decoders here. */
 
-#define DECL(name) extern r_device name;
+#define DECL(name) extern r_device const name;
+DECL(flex_decoder)
 DEVICES
 #undef DECL
 

src/devices/blueline.c

@@ -397,7 +397,7 @@ static char const *const output_fields[] = {
 
 r_device const blueline;
 
-static r_device *blueline_create(char *arg)
+static r_device *blueline_create(char const *arg)
 {
     r_device *r_dev = decoder_create(&blueline, sizeof(struct blueline_stateful_context));
     if (!r_dev) {

src/devices/fineoffset.c

@@ -15,7 +15,7 @@
 
 r_device const fineoffset_WH2;
 
-static r_device *fineoffset_WH2_create(char *arg)
+static r_device *fineoffset_WH2_create(char const *arg)
 {
     if (arg && !strcmp(arg, "no-wh5")) {
         r_device *r_dev = decoder_create(&fineoffset_WH2, sizeof(int));

src/devices/flex.c

@@ -12,6 +12,7 @@
 #include "decoder.h"
 #include "optparse.h"
 #include "fatal.h"
+#include "rtl_433_devices.h"
 #include <stdlib.h>
 
 enum uart_mode {
@@ -657,10 +658,23 @@ static unsigned parse_uart_mode(char const *str)
     return 0;
 }
 
-// NOTE: this is declared in rtl_433.c also.
-r_device *flex_create_device(char *spec);
+static void flex_destroy_device(r_device *dev)
+{
+    struct flex_params *params = decoder_user_data(dev);
+    free((void *)dev->name);
+    free(params->name);
+    for (int idx=0; idx < GETTER_SLOTS; ++idx) {
+        free(params->getter[idx].name);
+        free(params->getter[idx].format);
+        for (int map_idx=0; map_idx < GETTER_MAP_SLOTS; ++map_idx) {
+            free(params->getter[idx].map[map_idx].val);
+        }
+    }
+    free(dev->decode_ctx);
+    free(dev);
+}
 
-r_device *flex_create_device(char *spec)
+static r_device *flex_create_device(char const *spec)
 {
     if (!spec || !*spec || *spec == '?' || !strncasecmp(spec, "help", strlen(spec))) {
         help();
@@ -673,15 +687,19 @@ r_device *flex_create_device(char *spec)
     struct flex_params *params = decoder_user_data(dev);
     int get_count = 0;
 
-    spec = strdup(spec);
-    if (!spec)
+    char * mutable_spec = strdup(spec); // spec will be mutated by getkwargs()
+    if (!mutable_spec)
         FATAL_STRDUP("flex_create_device()");
+    spec = mutable_spec;
 
+    // Copy const fields from static global struct
+    dev->create_fn = flex_create_device;
+    dev->destroy_fn = flex_destroy_device;
     dev->decode_fn = flex_callback;
     dev->fields = output_fields;
 
     char *key, *val;
-    while (getkwargs(&spec, &key, &val)) {
+    while (getkwargs(&mutable_spec, &key, &val)) {
         key = remove_ws(key);
         val = trim_ws(val);
 
@@ -858,6 +876,14 @@ r_device *flex_create_device(char *spec)
                 params->min_rows, params->min_bits, params->min_repeats, params->invert, params->reflect, params->match_len, params->preamble_len);
     */
 
-    free(spec);
+    free((void*)spec);
     return dev;
 }
+
+r_device const flex_decoder = {
+        .name        = "General purpose decoder",
+        .create_fn   = &flex_create_device,
+        .destroy_fn  = &flex_destroy_device,
+        .decode_fn   = &flex_callback,
+        .fields      = output_fields,
+};

src/r_api.c

@@ -280,6 +280,9 @@ void register_protocol(r_cfg_t *cfg, r_device *r_dev, char *arg)
     r_device *p;
     if (r_dev->create_fn) {
         p = r_dev->create_fn(arg);
+        // create_fn() doesn't know or apply the main() assigned
+        // protocol number
+        p->protocol_num = r_dev->protocol_num;
     }
     else {
         if (arg && *arg) {
@@ -307,9 +310,13 @@ void register_protocol(r_cfg_t *cfg, r_device *r_dev, char *arg)
 
 void free_protocol(r_device *r_dev)
 {
-    // free(r_dev->name);
-    free(r_dev->decode_ctx);
-    free(r_dev);
+    if (r_dev->destroy_fn) {
+        r_dev->destroy_fn(r_dev);
+    }
+    else {
+        free(r_dev->decode_ctx);
+        free(r_dev);
+    }
 }
 
 void unregister_protocol(r_cfg_t *cfg, r_device *r_dev)

src/rtl_433.c

@@ -55,6 +55,7 @@
 #include "write_sigrok.h"
 #include "sigmf.h"
 #include "mongoose.h"
+#include "rtl_433_devices.h"
 
 #ifdef _WIN32
 #include <io.h>
@@ -123,8 +124,6 @@ static void delay_timer_wait(delay_timer_t *delay_timer, unsigned delay_us)
         usleep(delay_us - elapsed_us);
 }
 
-r_device *flex_create_device(char *spec); // maybe put this in some header file?
-
 static void print_version(void)
 {
     fprintf(stderr, "%s\n", version_string());
@@ -1225,11 +1224,7 @@ static void parse_conf_option(r_cfg_t *cfg, int opt, char *arg)
         }
         break;
     case 'X':
-        if (!arg)
-            flex_create_device(NULL);
-
-        flex_device = flex_create_device(arg);
-        register_protocol(cfg, flex_device, "");
+        register_protocol(cfg, &flex_decoder, arg);
         break;
     case 'q':
         fprintf(stderr, "quiet option (-q) is default and deprecated. See -v to increase verbosity\n");

tests/CMakeLists.txt

@@ -33,6 +33,24 @@ endforeach(testSrc)
 # Define integration tests
 ########################################################################
 add_test(rtl_433_help ../src/rtl_433 -h)
+add_test(flex_decoder_help ../src/rtl_433 -X)
+add_test(flex_decoder bash -c
+    "touch empty_file.cu8 && \
+    ../src/rtl_433 \
+        -X n=name,m=FSK_PCM,s=10,l=10,r=10240 \
+        -r empty_file.cu8")
+# Borrowed from: https://github.com/merbanan/rtl_433/pull/786#issuecomment-410513334
+add_test(flex_decoder_w_getter bash -c
+    "touch empty_file.cu8 && \
+    ../src/rtl_433 \
+        -X 'n=name,m=FSK_PCM,s=10,l=10,r=10240,get=@20:{12}0x7f:status:[10:pir 14:open 7:close 11:tamper 0xf:battery_low]' \
+        -r empty_file.cu8")
+# Borrowed from: https://github.com/merbanan/rtl_433/pull/1532#issue-728103584
+add_test(flex_decoder_w_getter_and_format_specifier bash -c
+    "touch empty_file.cu8 && \
+    ../src/rtl_433 \
+        -X 'n=name,m=FSK_PCM,s=10,l=10,r=10240,get=@2:{32}:code:%x' \
+        -r empty_file.cu8")
 
 ########################################################################
 # Define style checks