Free userdata if plugin_register_complex_read() has failed.
#2349
Conversation
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
8eb6404 to
0bdfdca
Compare
0bdfdca to
53e7e60
Compare
|
Hi Florian, Thanks! Rebased, passed through clang-format. cc @octo |
octo
reviewed
Jul 24, 2017
There was a problem hiding this comment.
Hi @rpv-tomsk,
thanks for this! This looks good overall, just one small issue that we should fix before merging.
Best regards,
—octo
src/daemon/plugin.c
Outdated
| @@ -345,6 +349,7 @@ static int create_register_callback(llist_t **list, /* {{{ */ | |||
|
|
|||
| 8000 | cf = calloc(1, sizeof(*cf)); | ||
| if (cf == NULL) { | |||
| free_userdata(*ud); | |||
There was a problem hiding this comment.
This dereferences ud without checking for NULL. I'd suggest to change free_userdata to take a user_data_t* (i.e. a pointer) instead and check for NULL there.
There was a problem hiding this comment.
Updated, thanks!
Things become more clear after this change.
added 2 commits
July 24, 2017 13:31
…stering it. The message `Check for duplicate "LoadPlugin" lines` is obsolete, as there is a special check in `plugin_load()` for this case.
e8b649e to
ff0d8b0
Compare
If `plugin_register_complex_read()` has failed, then `cldap_free` is called which set pointer to NULL. That pointer is accessed by `cldap_shutdown()` callback later.
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.
Citation from Collectd wiki (https://collectd.org/wiki/index.php/User_data_t):
There are several functions which register callbacks and has
user_data_targument:plugin_register_write(), plugin_register_log(), plugin_register_notification(), plugin_register_flush()- which callscreate_register_callback().plugin_register_complex_read(), which implemented in other way.Most of plugins expects what free() will happen if registration failed.
For first functions group, in case of failure,
create_register_callback()callsdestroy_callback()which frees userdata.But there was no code in
plugin_register_complex_read()for userdata free.That code was added, and some plugins (and Collectd itself) code was adjusted to new behaviour.