ticket 30984 control refactor #1505
Merged
Conversation
Pull Request Test Coverage Report for Build 7489
|
teor2345
suggested changes
Nov 6, 2019
This looks great!
I have a function name change suggestion, and a request for missing function docs.
| control_reply_add_str(reply, 250, "OK"); | ||
| } | ||
|
|
||
| void |
Doc?
And the remaining functions in this file.
src/feature/control/control_proto.c
Outdated
| * @param val value | ||
| */ | ||
| void | ||
| control_reply_add_1kv(smartlist_t *reply, int code, int flags, |
This name is hard to read and type, because "1" and "l" and "I" look similar:
Suggested change
| control_reply_add_1kv(smartlist_t *reply, int code, int flags, | |
| control_reply_add_single_kv(smartlist_t *reply, int code, int flags, |
Add a check for '=' characters in needs_escape(). This simplifies the logic in kvline_can_encode_lines(). Part of #30984.
Add the KV_RAW flag to kvline_encode(). This allows generation of output that is compatible with some quirks of the control protocol. Part of #30984.
Part of #30984.
In handle_control_getconf(), use the new control reply line abstraction to simplify output generation. Previously, this function explicitly checked for whether it should generate a MidReplyLine or an EndReplyLine. control_write_reply_lines() now abstracts this check. Part of #30984.
Factor out the parts of handle_control_protocolinfo() that assemble the AUTHMETHODS and COOKIEFILE strings. Part of #30984.
Simplify handle_control_protocolinfo() by using the new reply line abstraction. Part of #30984.
Simplify handle_control_getinfo() by using the new reply lines abstraction. Previously, this function explicitly checked for whether it should generate a MidReplyLine, a DataReplyLine, or an EndReplyLine. control_write_reply_lines() now abstracts this check. Part of #30984.
Part of ticket 30984.
teor2345
reviewed
Dec 9, 2019
src/feature/control/control_cmd.c
Outdated
| } | ||
|
|
||
| while (answer) { | ||
| config_line_t *next; | ||
| control_reply_add_1kv(answers, 250, KV_RAW, answer->key, | ||
| control_reply_add_one_kv(answers, 250, KV_RAW, answer->key, | ||
| answer->value); |
Nit: If you want to fix the spacing on this line before we merge, that would be good. Leaving it would also be ok.
Part of ticket 30984.
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.
No description provided.
The text was updated successfully, but these errors were encountered: