refactor control.c output to be more abstract

Lots of code explicitly assembles control protocol responses, including the numeric response codes, the line separators, etc.

We should abstract this better to simplify code and for greater consistency in the protocol.

changed milestone to %Tor: 0.4.1.x-final

Trac:
Parent Ticket: #29210 (moved)

added component::core tor/tor control-port dgoulet-merge milestone::Tor: 0.4.1.x-final owner::catalyst parent::29210 points::3 priority::medium resolution::fixed reviewer::nickm severity::normal sponsor::31-can status::closed technical-debt type::task labels

WIP branch at https://github.com/tlyu/tor/tree/ticket30007

older WIP branch (superseded by recent control.c refactor) at https://github.com/tlyu/tor/tree/control-proto

The new iteration might use a Coccinelle script to rewrite calls to formatting functions.

The WIP branch at https://github.com/tlyu/tor/tree/ticket30007 now has a Coccinelle script that I ran to migrate existing code to using the new abstractions. It's almost ready to review. Once Nick rebases the #30091 (moved) branch I based it on, I'll probably make a WIP pull request to check Coveralls.

There are a few places that still explicitly use connection_write_str_to_buf() and connection_buf_add() to send replies to the control port, but they're somewhat more difficult to change. I'm tempted to defer them to a later ticket.

In particular, these could benefit from additional refactoring that includes helper functions that collect a list of strings (or key-value pairs, etc.) to write and automatically takes care of the 250-some_text vs 250 some_text, etc. distinction.

Pull request at https://github.com/torproject/tor/pull/956

FWIW, I am happy to merge_ready this once the control-cmd branch is merge-readied. I want to do a final fine-grained review, but the broad strokes of this branch look great.

Trac:
Reviewer: N/A to nickm
Keywords: technical-debt deleted, technical-debt control-port added
Milestone: Tor: unspecified to Tor: 0.4.1.x-final

I noticed it needs a changes file and a WIP notation deleted from a commit message. I also should rebase it after #30091 (moved) is merged and verify the Coccinelle output again.

Trac:
Status: assigned to needs_revision

Rebased version at https://github.com/torproject/tor/pull/984

Trac:
Status: needs_revision to needs_review

This still lgtm; let's merge it once #30091 (moved) is in.

Force-pushed that pull request. Also re-verified the Coccinelle output. Waiting for CI to finish.

Replying to catalyst:

Force-pushed that pull request. Also re-verified the Coccinelle output. Waiting for CI to finish. CI has passed.

Trac:
Status: needs_review to merge_ready
Keywords: technical-debt control-port deleted, technical-debt control-port dgoulet-merge added

Merged!

Trac:
Status: merge_ready to closed
Resolution: N/A to fixed

closed

changed time estimate to 24h

mentioned in issue #30889 (moved)

mentioned in issue #29210 (moved)

moved to tpo/core/tor#30007 (closed)

mentioned in issue tpo/core/tor#30889 (closed)