Provide initvalue in GETINFO config/names

To handle torrc config options from Vidalia it would be useful to me to have the default values with each option printed by config/names.

Attached is a simple patch to handle this.

changed milestone to %Tor: 0.2.4.x-final

added component::core tor/tor milestone::Tor: 0.2.4.x-final priority::medium resolution::implemented status::closed tor-client type::enhancement labels

Trac:
0001-Provide-init-values-for-config-names-getinfo-control.patch

Trac:
Status: new to needs_review

This patch doesn't apply to current origin/master . Please push a Git branch instead.

Use esc_for_log to quote var->initvalue when it exists (instead of wrapping it with double quotes), and distinguish a NULL initvalue from an empty-string initvalue.

Trac:
Status: needs_review to needs_revision

Trac:
Milestone: N/A to Tor: 0.2.4.x-final

Ok, updated based on rransom's comments in my branch bug4971_initvalues.

Trac:
Status: needs_revision to needs_review

From the documentation comment on esc_for_log in src/or/util.c:

/** Allocate and return a new string representing the contents of <b>s</b>,
 * surrounded by quotes and using standard C escapes.

So your branch leaks initvalue.

There are at least three ways to avoid freeing the constant string "NULL". One is to tor_strdup it in an else clause. Another is to use the tor_strdup("(null)") which esc_for_log returns if its argument is NULL as your representation of a NULL initvalue. Another is to stick a second copy of the pointer which esc_for_log returns into another variable which can always be passed to tor_free (because it always holds a pointer to a temporary object or NULL).

Does anything else parse the output of GETINFO config/names? Does control-spec.txt specify its output format?

Other than that, the code change looks good.

Trac:
Status: needs_review to needs_revision

Does anything else parse the output of GETINFO config/names?

Yup, arm does. https://gitweb.torproject.org/arm.git/blob/HEAD:/src/cli/configPanel.py#l213 https://gitweb.torproject.org/arm.git/blob/HEAD:/src/util/torConfig.py#l182 https://gitweb.torproject.org/arm.git/blob/HEAD:/src/util/torConfig.py#l359 https://gitweb.torproject.org/arm.git/blob/HEAD:/src/util/torInterpretor.py#l317

I don't mind changing it, so please reassign this ticket to me after a tor patch is merged.

Does control-spec.txt specify its output format?

Yes it does. https://gitweb.torproject.org/torspec.git/blob/HEAD:/control-spec.txt#l610

I don't like the idea of the initial value being at the end since it would then make this...

OptionName SP OptionType [ SP Documentation ][ SP InitialValue ] CRLF

Multiple optional arguments, especially when the first is defined as 'Text', means that controllers can't differentiate Documentation from the InitialValue. We could redefine the Documentation's Text type as not having quotes but from a parsing perspective... ewww.

Replying to atagar:

Does anything else parse the output of GETINFO config/names?

Yup, arm does. https://gitweb.torproject.org/arm.git/blob/HEAD:/src/cli/configPanel.py#l213 https://gitweb.torproject.org/arm.git/blob/HEAD:/src/util/torConfig.py#l182 https://gitweb.torproject.org/arm.git/blob/HEAD:/src/util/torConfig.py#l359 https://gitweb.torproject.org/arm.git/blob/HEAD:/src/util/torInterpretor.py#l317

I don't mind changing it, so please reassign this ticket to me after a tor patch is merged.

No. Any arm change should have a separate ticket.

Does control-spec.txt specify its output format?

Yes it does. https://gitweb.torproject.org/torspec.git/blob/HEAD:/control-spec.txt#l610

I don't like the idea of the initial value being at the end since it would then make this...

OptionName SP OptionType [ SP Documentation ][ SP InitialValue ] CRLF

Multiple optional arguments, especially when the first is defined as 'Text', means that controllers can't differentiate Documentation from the InitialValue. We could redefine the Documentation's Text type as not having quotes but from a parsing perspective... ewww.

No version of Tor will support both Documentation and InitialValue. If Documentation was removed in Tor 0.2.2.x, arm (and other controllers) should simply drop support for it.

It seems to me that if we want to change the format and semantics of an existing option in a way that the current spec doesn't permit, and if doing so will break current controllers, it might be better just to provide a new GETINFO key and call it something other than "config/names".

Here's a branch that implements a "GETINFO config/defaults" and simply doesn't list anything which has a NULL initvalue (which is a separate commit if you don't want that). This would need a torspec change too, which I can do if this is acceptable.

https://github.com/meejah/tor/commits/4971/

Trac:
Status: needs_revision to needs_review

looks good; we should merge this once 0.2.4.x is open.

Minor stuff it needs:

• "make check-spaces" should pass.
• Tor is indented in K&R style, so we write:

   } else if (condition) {

and not

   }
   else if (condition)
   {

• We'll need a changes file, and a patch to control-spec.txt.

I pushed changes for the K&R and whitespace -- don't know what was up with my emacs. Also, a change for torspec:

https://github.com/meejah/torspec/tree/4971

There is a changes/bug4971 in the branch:

https://github.com/meejah/tor/blob/4971/changes/bug4971

Is that what you mean by changes file?

Rebased the above branch.

I tweaked it just a little more (by removing one of the redundant blocks in config.c, and splitting a long line), then squashed and merged into master, which will become Tor 0.2.4.

You can see the changes in "ticket4971" in my public repository.

Thanks again!

Trac:
Status: needs_review to closed
Resolution: N/A to implemented

Trac:
Keywords: N/A deleted, tor-client added

Trac:
Component: Tor Client to Tor

closed

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