Opened 6 years ago

Closed 5 months ago

Last modified 4 months ago

#4998 closed defect (implemented)

MyFamily as a list

Reported by: weasel Owned by: Jigsaw52
Priority: Medium Milestone: Tor: 0.3.1.x-final
Component: Core Tor/Tor Version: Tor: 0.2.3.11-alpha
Severity: Normal Keywords: tor-relay lorax easy intro
Cc: Actual Points:
Parent ID: #15060 Points:
Reviewer: Sponsor:

Description

Hi,

MyFamily currently is a config string. It'd be nice if it was a line list like RecommendedVersions as that would make configuration files a bit more readable.

Child Tickets

Attachments (1)

config.patch (556 bytes) - added by pingl 13 months ago.
config.c patch

Download all attachments as: .zip

Change History (19)

comment:1 Changed 6 years ago by nickm

Milestone: Tor: unspecified

Sounds like a plausible idea to me. It would need to be a listlist of comma-separated entries, for backward compatibility.

comment:2 Changed 5 years ago by nickm

Keywords: tor-relay added

comment:3 Changed 5 years ago by nickm

Component: Tor RelayTor

comment:4 Changed 3 years ago by nickm

Parent ID: #15060

comment:5 Changed 2 years ago by nickm

Keywords: lorax added

comment:6 Changed 19 months ago by teor

Keywords: easy intro added
Severity: Normal

Changed 13 months ago by pingl

Attachment: config.patch added

config.c patch

comment:7 Changed 12 months ago by cypherpunks

Status: newneeds_review

comment:8 Changed 12 months ago by teor

Status: needs_reviewneeds_revision

The code that processes MyFamily still expects it to be a char * - this patch changes it to a smartlist * containing char *, but doesn't change how it's processed.

comment:9 Changed 5 months ago by Jigsaw52

Status: needs_revisionneeds_review

comment:10 Changed 5 months ago by nickm

Milestone: Tor: unspecifiedTor: 0.3.1.x-final

comment:11 Changed 5 months ago by nickm

Status: needs_reviewneeds_revision

Getting there!

But it looks like this patch makes it so Tor would no longer accept the old format. That's not right, though: we need to make sure that the there can be multiple lines *and* multiple options on each line. Otherwise all old MyFamily lines will stop working.

comment:12 Changed 5 months ago by Jigsaw52

Status: needs_revisionneeds_review

I does accept both multiple lines and multiple options on the same line. I have changed the test to actually test both cases and tested manually with the following lines on torrc:

MyFamily $1111111111111111111111111111111111111111, 1111111111111111111111111111111111111112, $1111111111111111111111111111111111111113
MyFamily 1111111111111111111111111111111111111114
MyFamily $1111111111111111111111111111111111111115

However, I did change the man page to mention only the new format. I did that to encourage the use of the new format. I can change the man page to mention both formats if you think it is better.

comment:13 Changed 5 months ago by Jigsaw52

Owner: set to Jigsaw52
Status: needs_reviewassigned

comment:14 Changed 5 months ago by Jigsaw52

Status: assignedneeds_review

comment:15 Changed 5 months ago by nickm

Status: needs_reviewmerge_ready

Oh, I see what you did. Yes, you're correct there. The code looks like it should work.

One thing that we try not to do any more, though: we try not to rewrite values that could get written back into the torrc by a SAVECONF command. What we usually do instead now is, when we want to normalize a configuration value, normalize it into a separate field of or_options_t. What do you think of the patch I've made on top of your branch, in my-family-list-fix-4498 in my public repository? The URL is

https://gitweb.torproject.org/nickm/tor.git/log/?h=my-family-list-fix-4498

comment:16 Changed 5 months ago by Jigsaw52

Thanks for the explanation.
I checked out out your branch and tested it and everything seems fine.

comment:17 Changed 5 months ago by nickm

Resolution: implemented
Status: merge_readyclosed

Excellent; merged to master. Thanks for the patches!

comment:18 Changed 4 months ago by arma

Alas, this patch seems to have caused #22368 (a double free bug).

Note: See TracTickets for help on using tickets.