Make configuration validation function return validation error message#193
Make configuration validation function return validation error message#193danghai wants to merge 1 commit into
Conversation
spbnick
left a comment
spbnick
left a comment
There was a problem hiding this comment.
This looks good, I only have one style comment.
Also, I think we should add similar assertions at the entry of the functions which accept configuration. Could you see if we can do it? Thanks, Hai!
| /* Check validate the config */ | ||
| assert(tlog_play_conf_validate(NULL, | ||
| conf, | ||
| TLOG_CONF_ORIGIN_ARGS) == TLOG_RC_OK); |
There was a problem hiding this comment.
Could you wrap it a bit differently, so it takes less lines? Here and elsewhere? E.g.:
assert(tlog_play_conf_validate(NULL, conf, TLOG_CONF_ORIGIN_ARGS) ==
TLOG_RC_OK);
|
I have updated it |
spbnick
left a comment
spbnick
left a comment
There was a problem hiding this comment.
Aside from the inline comment, this commit subject doesn't make sense. Please figure out what we're doing and why we're doing it and fix the subject.
| assert(argv != NULL); | ||
| /* Check validate the config */ | ||
| assert(tlog_play_conf_validate(NULL, conf, TLOG_CONF_ORIGIN_ARGS) == | ||
| TLOG_RC_OK); |
There was a problem hiding this comment.
This wouldn't make sense, we don't need to validate a NULL pointer we just initialized. This function doesn't need to validate the configuration it receives, as it doesn't receive any. The same for the rest of those.
3 participants
Hi Nick, PR for issue #41 to make configuration validation function return validation error message by using
assert.