minor: Fix flex device memory leak during destruction (#3350) #3351
Conversation
dandwyer
changed the title
| 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); |
There was a problem hiding this comment.
Made param const since only 3 decoders use this method and flex.c was confusingly mutating this param.
| /* Add new decoders here. */ | ||
|
|
||
| #define DECL(name) extern r_device name; | ||
| #define DECL(name) extern r_device const name; |
|
|
||
| #define DECL(name) extern r_device name; | ||
| #define DECL(name) extern r_device const name; | ||
| DECL(flex_decoder) |
There was a problem hiding this comment.
I had made a flex.h header in previous revision, but ultimately felt like it was best to be consistent w/ other decoders to extent possible.
|
|
||
| char *key, *val; | ||
| while (getkwargs(&spec, &key, &val)) { | ||
| while (getkwargs(&mutable_spec, &key, &val)) { |
There was a problem hiding this comment.
Every iteration of getkwargs advances the mutable_spec pointer to the next key/value pair, which was causing the call to free(spec); to not be freeing the strdup allocation in the code on the left.
|
|
||
| r_device const flex_decoder = { | ||
| .name = "General purpose decoder", | ||
| .create_fn = &flex_create_device, |
There was a problem hiding this comment.
In theory, could likely have struct only have create_fn and let the create method populate everything else.
| free(r_dev->decode_ctx); | ||
| free(r_dev); | ||
| if (r_dev->destroy_fn) { | ||
| r_dev->destroy_fn(r_dev); |
There was a problem hiding this comment.
flex.c is only decoder w/ destroy_fn, but this seemed more extensible than giving special treatment for protocol 0.
| // create_fn() doesn't know or apply the main() assigned | ||
| // protocol number | ||
| p->protocol_num = r_dev->protocol_num; |
There was a problem hiding this comment.
I had a different iteration where free_protocol was using protocol_num to give special treatment to 0. Although this isn't strictly necessary for this revision, it was a point of confusion for me why protocols w/ create_fn were having zeroed protocol_num before.
| 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") |
There was a problem hiding this comment.
This induces the memory error: https://github.com/dandwyer/rtl_433/actions/runs/17200738585/job/48790690802#step:9:387
zuckschwerdt
force-pushed
the
master
branch
2 times, most recently
from
8d2ffe8 to
a796732
Compare
1 participant
This PR is meant to address: #3350
build: Add memory leak detection in "Build Analyze Check" GitHub Actionhighlights the issue as can be seen by the workflow in my forked repo: https://github.com/dandwyer/rtl_433/actions/runs/17200738585/job/48790690802#step:9:387minor: Fix flex device memory leak during destructionaddresses the issue as can be seen here: https://github.com/dandwyer/rtl_433/actions/runs/17200928948