Document a stable tor main function

For the usage in the mobile context it's useful to have a documented tor main function to call to start it and exposed as C headers.

changed milestone to %Tor: 0.3.3.x-final in legacy/trac

added component::core tor/tor in Legacy / Trac milestone::Tor: 0.3.3.x-final in Legacy / Trac owner::nickm in Legacy / Trac parent::23684 in Legacy / Trac priority::medium in Legacy / Trac resolution::implemented in Legacy / Trac review-group-24 in Legacy / Trac s8-api in Legacy / Trac severity::normal in Legacy / Trac sponsor::8 in Legacy / Trac status::closed in Legacy / Trac tor-mobile in Legacy / Trac type::enhancement in Legacy / Trac labels

Trac:
Type: defect to enhancement

Trac:
Cc: darkk, brade, mcs, nickm, ahf to darkk, brade, mcs, nickm, ahf, mike@tig.as

Trac:
Cc: darkk, brade, mcs, nickm, ahf, mike@tig.as to darkk, brade, mcs, nickm, ahf, mike@tig.as, sbs

Trac:
Status: new to accepted
Owner: N/A to nickm

See branch tor_api in my tor repository. Probably needs some changes!

Trac:
Status: accepted to needs_review

review-group-24 is now open.

Trac:
Keywords: N/A deleted, review-group-24 added

Trac:
Sponsor: N/A to Sponsor8

72b5e4a2db4282002fe50e11c2b8a79e108d30f8 looks fine.

In 8dd957cb8ae5c82da576e7720b1bb3b846089f98 the comment:

 * This interface is intended for use by programs that need to link Tor as
 * a library, and launch it in a separate thread.  If you have the ability
 * to run Tor as a separate process, you probably should.
 **/

reads a bit to me that this API is only useful if you have Tor running in a thread (but not as a process) - isn't it useful in both cases?

Otherwise this commit looks good as well. Nice with a documented, public, header! :-)

a7c126f65438a8dcde5663a731f0f8528e480bcc looks good.

Trac:
Status: needs_review to merge_ready

isn't it useful in both cases?

If you can run Tor as a separate process, I think you should probably just use exec*() to launch it, rather than having it linked in as part of your binary. I've tried to clarify in a fixup commit. Does it look better now?

Also, I think I should wait for the mobile people to comment too, so we know if they like the API

Yep, the fixup looks good.

Let's await and see what the mobile people say.

Trac:
Keywords: N/A deleted, s8-api added

sbs and I looked into this and agree that it looks good.

Some minor nitpicks and comments we have are the following:

• What is the purpose of tools/tor_runner.c? We suspect it's some sort of test to see if the API works, but were unclear about what it's exact purpose (and usage) is.
• It would be useful to maybe add to the docstring of tor_run_main explaining how to construct tor_main_configuration_t
• Related to the previous point, it would be useful to document what are the "best practices" in terms of initial configuration options to pass to tor_run_main when used as part of a library (for example, setting OwningControllerProcess or not). Good pointers can be found in txtorcon: https://github.com/meejah/txtorcon/blob/master/txtorcon/controller.py#L76.

I don't think these are blocking to merging this branch, but if you consider these things useful, maybe it would be appropriate to file some tickets for them (if you would rather postpone this).

Thanks for the quick turnaround on this, it's greatly appreciated!

Replying to hellais:

sbs and I looked into this and agree that it looks good.

Some minor nitpicks and comments we have are the following:

• What is the purpose of tools/tor_runner.c? We suspect it's some sort of test to see if the API works, but were unclear about what it's exact purpose (and usage) is.

Good point -- I should add some comments. It is meant to make a library that provides the same API as tor_run_main(), but which launches Tor in a separate process, so that you can write code that will work either way.

• It would be useful to maybe add to the docstring of tor_run_main explaining how to construct tor_main_configuration_t

Will do.

• Related to the previous point, it would be useful to document what are the "best practices" in terms of initial configuration options to pass to tor_run_main when used as part of a library (for example, setting OwningControllerProcess or not). Good pointers can be found in txtorcon: https://github.com/meejah/txtorcon/blob/master/txtorcon/controller.py#L76.

I think this will have to be a separate ticket. Many of those ideas should IMO become other options that you can set on tor_main_configuration_t.

I don't think these are blocking to merging this branch, but if you consider these things useful, maybe it would be appropriate to file some tickets for them (if you would rather postpone this).

Thanks for the quick turnaround on this, it's greatly appreciated!

Fixed and merged to master!

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

closed

mentioned in issue legacy/trac#23900 (moved)

moved from legacy/trac#23845 (moved)

added Feature label and removed 1 deleted label

removed 1 deleted label