-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Split up parse config function #739
Split up parse config function #739
Conversation
80c6499
to
30ecf62
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally looks great, maybe just 2 suggestions for different name
Relates to: eclipse-bluechi#678 The parse functions agent_parse_config and controller_parse_config initialize the config, set default values there, load configuration from all sources and sets the loaded values to the agent/controller fields. This is a bit much, therefore, split setting the values out to a dedicated function. In addition to a smaller, easier to read function it also simplifies writing unit tests. Signed-off-by: Michael Engel <mengel@redhat.com>
Relates to: eclipse-bluechi#678 Added unit tests for the previously split out function to set the values from the loaded configuration to the agent/controller fields. Signed-off-by: Michael Engel <mengel@redhat.com>
Two leaks are fixed: - agent_unref did not free the peer_socket_options - unit_infos was allocated twice Signed-off-by: Michael Engel <mengel@redhat.com>
Signed-off-by: Michael Engel <mengel@redhat.com>
df15764
to
71662b0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There is still an error in the coverage collection causing it to drop - we are on it. |
Relates to: #678
The parse functions agent_parse_config and controller_parse_config initialize the config, set default values there, load configuration from all sources and sets the loaded values to the agent/controller fields. This is a bit much, therefore, split setting the values out to a dedicated function. In addition to a smaller, easier to read function it also simplifies writing unit tests - which are also added in this PR.
It also fixes two memory leaks: