testsuite: add valgrind coverage #120
Merged
Conversation
This file contains 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
Problem: powermand does not call destructors unless terminated by a signal. Instead of calling destructors only from the signal handler, call them upon exit of the select loop, and have the signal handler wake up the select loop by writing to a pipe. Fixes chaos#119
Problem: the config parser leaks the list of listen addresses. ==13356== 40 bytes in 1 blocks are possibly lost in loss record 1 of 14 ==13356== at 0x4849E4C: malloc (vg_replace_malloc.c:307) ==13356== by 0x120647: xmalloc (xmalloc.c:53) ==13356== by 0x12098B: xstrdup (xmalloc.c:115) ==13356== by 0x114FC7: conf_add_listen (parse_util.c:219) ==13356== by 0x114723: yyparse (parse_tab.y:170) ==13356== by 0x114B4B: parse_config_file (parse_tab.y:370) ==13356== by 0x114CD3: conf_init (parse_util.c:92) ==13356== by 0x10AE9F: main (powermand.c:124) Destroy the listen address list in conf_fini().
Problem: the list of aliases is not destroyed when the config parser is destroyed. ==14147== 128 bytes in 1 blocks are possibly lost in loss record 8 of 19 ==14147== at 0x484C164: calloc (vg_replace_malloc.c:760) ==14147== by 0x115CAF: hostlist_new (hostlist.c:1069) ==14147== by 0x1191BF: _hostlist_create_bracketed.constprop.0 (hostlist.c:1500) ==14147== by 0x115243: _alias_create (parse_util.c:328) ==14147== by 0x115243: conf_add_alias (parse_util.c:344) ==14147== by 0x1147EB: makeAlias (parse_tab.y:713) ==14147== by 0x1147EB: yyparse (parse_tab.y:187) ==14147== by 0x114B4B: parse_config_file (parse_tab.y:370) ==14147== by 0x114CD3: conf_init (parse_util.c:92) ==14147== by 0x10AE9F: main (powermand.c:124) Destroy the alias list in conf_fini().
Problem: a comment in parse_util.c::conf_init() refers to source files and data structures that no longer exist. Drop those references from the comment.
Problem: _clear_current_spec() is a function that just zero-initializes a struct. Replace _clear_current_spec() with memset(0) and rely on BSS zero-initialization to zero the structure at startup.
Problem: _copy_current_spec() copies an array that was already copied by structure assignment Don't do that.
problem: destroyStmt() doesn't free plug_name which was allocated in makeStmt(). Add missing free.
Problem: destroyStmt() destroys stmt->u.foreach.stmts when it should destroy stmt->u.ifonoff.stmts. This happens to work because the structs are aligned, but it is fragile. Destroy the correct member.
Problem: there is not valgrind coverage for powermand. Add a test script. Currently the test is expected to fail, but at least now we know some things that need to be fixed.
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.
This adds a valgrind test and fixes a few memory leaks but not all of them.The test currently is an expected failure, but it can be run with
-dand the failures can be picked out of the trash. I suppose we could merge this as is and pick off the rest of the leaks as time permits. (I tried but didn't find anything obvious)