Opened 6 years ago

Last modified 19 months ago

#8351 needs_revision enhancement

Refactor our controller-command/torrc-option processing logic into a data-driven function

Reported by: nickm Owned by:
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-client, refactoring, nickm-patch torrc
Cc: yawning, catalyst Actual Points:
Parent ID: Points: medium
Reviewer: Sponsor:

Description

Throughout our controller protocol and our configuration code, we process things with more or less the following metaformat:

 Line = PositionalOptions | KWDOptions | BothOptions
 BothOptions = PositionalOptions SP KWDOptions
 PositionalOptions = PosOption | PosOption SP PositionalOptions
 KWDOptions = KWDOption | KWDOption SP KWDOptions

 PosOption = QuotedOption | NonSpace
 QuotedOption = QString
 NonSpace = PChar*
 
 KWDOption = KWDName "=" PosOption
 
 PChar = a printing, nonspace character.
 QString = DOUBLE_QUOTE QContent* DOUBLE_QUOTE
 


 SP = One or more whitespace characters

We should have one generic function that parses a config line or command into a list of positional and keyword arguments, and another that takes such a list, typechecks it, and converts it into a structure (in the same way that config_assign does). The latter function could even _be_ config_assign.

In theory, this should make our parsing more convenient, and remove a bunch of duplicate code.

(Did I already open a ticket for this? I'm not seeing it if so.)

Child Tickets

Change History (19)

comment:1 Changed 6 years ago by nickm

Status: newneeds_revision

I did a sketch implementation the other day in branch "ticket8351". It seems like it'll the core of something good. I haven't yet done the code to make the controller/state/config code use it, since the obvious unit tests for that are things that will get way easier once we have a good mocking system. This is a fun one to come back to later.

comment:2 Changed 5 years ago by nickm

Milestone: Tor: 0.2.5.x-finalTor: 0.2.6.x-final

Moving nearly all needs_revision tickets into 0.2.6, as now untimely for 0.2.5.

comment:3 Changed 4 years ago by nickm

Keywords: 026-triaged-1 026-deferrable added

comment:4 Changed 4 years ago by nickm

Keywords: nickm-patch added

Add nickm-patch keyword to needs_revision tickets in 0.2.6 where (I think) I wrote the patch, so I know which ones are my responsibility to revise.

comment:5 Changed 4 years ago by nickm

Milestone: Tor: 0.2.6.x-finalTor: 0.2.???

comment:6 Changed 4 years ago by yawning

Cc: yawning added

comment:7 Changed 4 years ago by nickm

Milestone: Tor: 0.2.???Tor: 0.2.7.x-final

Pulling this (naughtily) into 0.2.7 because it might help with several other controller-related tickets.

comment:8 Changed 3 years ago by nickm

Milestone: Tor: 0.2.7.x-finalTor: 0.2.8.x-final

comment:9 Changed 3 years ago by nickm

Points: medium

comment:10 Changed 3 years ago by nickm

Keywords: pre028-patch added

comment:11 Changed 3 years ago by nickm

Milestone: Tor: 0.2.8.x-finalTor: 0.2.???

It is impossible that we will fix all 261 currently open 028 tickets before 028 releases. Time to move some out. This is my first pass through the "needs_revision" and "needs_information" tickets, looking for things to move to ???.

Note that in most cases, if these tickets get the requested revisions done in time for the 0.2.8 merge window, they could get considered for review and merge in 0.2.8.

comment:12 Changed 2 years ago by teor

Milestone: Tor: 0.2.???Tor: 0.3.???

Milestone renamed

comment:13 Changed 2 years ago by nickm

Keywords: tor-03-unspecified-201612 added
Milestone: Tor: 0.3.???Tor: unspecified

Finally admitting that 0.3.??? was a euphemism for Tor: unspecified all along.

comment:14 Changed 19 months ago by nickm

Keywords: tor-03-unspecified-201612 removed

Remove an old triaging keyword.

comment:15 Changed 19 months ago by nickm

Keywords: 026-triaged-1 removed

comment:16 Changed 19 months ago by nickm

Keywords: pre028-patch removed

comment:17 Changed 19 months ago by nickm

Keywords: 026-deferrable removed

comment:18 Changed 19 months ago by nickm

Keywords: torrc added
Severity: Normal

comment:19 Changed 19 months ago by catalyst

Cc: catalyst added
Note: See TracTickets for help on using tickets.