Fix some memory leaks#297
Merged
Merged
Conversation
* generate.c: free the options context allocated at the start of the main
function.
* openvswitch.c: make sure the GString "cmds" is free'd before the
function returns.
* parse.c: free the temporary list "values" before the function
returns.
* parse.c: parsed_defs is set to NULL preventing
netplan_parser_reset() to free the memory allocated for the
hash table. Not setting it to NULL is safe because all the data is moved to
another hash table, leaving parsed_defs with no elements. Not
nullifying parsed_defs here allows netplan_parser_reset() to free
it.
* types.c: in this particular case we g_strdup both key and value
strings when inserting them in the hash table in
parse.c:handle_generic_map(). Calling the destructor for both of
them will make sure all the memory is free'd. When using
free_hashtable_with_destructor in the future we'll need to make
sure both key and value are not poiting to the same place to avoid
a double free.
* util.c: release the glob_t allocated in find_yaml_glob().
* error.c: make sure the pointer 'line' is free'd after the last
loop iteration. Also, free error_context before returning.
* generate.c: free the error object if an error is returned at some point.
* parse-nm.c: free "ap" when ssid can't be found (caught by clang
build-scan).
* parse.c: filepath might be overwritten in some situations. Make
sure the old one is free'd.
* parse.c: netdef->vxlan might be overwritten in some situations.
Make sure the old one is free'd.
* types.c: free embedded_switch_mode when the netdef is reset.
slyon
approved these changes
Nov 21, 2022
Contributor
slyon
left a comment
There was a problem hiding this comment.
Thank you very much, LGTM!
I really like the systematic approach of searching for memory leaks using valgrind!
| if (def->ovs_settings.controller.addresses && def->ovs_settings.controller.addresses->len > 0) { | ||
| if (!write_ovs_bridge_controller_targets(settings, &(def->ovs_settings.controller), def->id, cmds, error)) | ||
| return FALSE; | ||
| return FALSE; |
Contributor
There was a problem hiding this comment.
Unrelated, but makes sense to fix this up, while touching the file!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The before and after can be seen by running the generate program with valgrind
assuming you have an etc/netplan in /tmp/fakeroot:
valgrind --leak-check=full ./generate -r /tmp/fakeroot/It can also be checked by compiling netplan with ASAN:
CFLAGS=-fsanitize=address meson setup build --prefix=/usrmeson compile -C build --verboseand then
./generate -r /tmp/fakeroot/Checklist
make checksuccessfully.make check-coverage).