Log the correct "auto" port number for listening sockets

[kjak]

When auto is used for the port number for a listening socket, the message that gets logged after opening the socket incorrectly says port 0 instead of the actual port being used.

A contrived config like this

ControlPort auto
SocksPort 127.0.0.1:auto
ORPort 127.0.0.1:auto
ORPort [::1]:auto

gives log messages like this

Jan 21 12:38:05.741 [notice] Opening Socks listener on 127.0.0.1:0
Jan 21 12:38:05.741 [notice] Socks listener listening on port 22840.
Jan 21 12:38:05.741 [notice] Opened Socks listener on 127.0.0.1:0
Jan 21 12:38:05.741 [notice] Opening Control listener on 127.0.0.1:0
Jan 21 12:38:05.741 [notice] Control listener listening on port 14414.
Jan 21 12:38:05.741 [notice] Opened Control listener on 127.0.0.1:0
Jan 21 12:38:05.741 [notice] Opening OR listener on 127.0.0.1:0
Jan 21 12:38:05.741 [notice] OR listener listening on port 19719.
Jan 21 12:38:05.741 [notice] Opened OR listener on 127.0.0.1:0
Jan 21 12:38:05.741 [notice] Opening OR listener on [::1]:0
Jan 21 12:38:05.742 [notice] OR listener listening on port 4298.
Jan 21 12:38:05.742 [notice] Opened OR listener on [::1]:0

An upcoming PR will fix this so that the log messages will have the actual port numbers like this

Jan 21 12:38:57.709 [notice] Opening Socks listener on 127.0.0.1:0
Jan 21 12:38:57.709 [notice] Socks listener listening on port 5236.
Jan 21 12:38:57.709 [notice] Opened Socks listener on 127.0.0.1:5236
Jan 21 12:38:57.709 [notice] Opening Control listener on 127.0.0.1:0
Jan 21 12:38:57.709 [notice] Control listener listening on port 14584.
Jan 21 12:38:57.709 [notice] Opened Control listener on 127.0.0.1:14584
Jan 21 12:38:57.709 [notice] Opening OR listener on 127.0.0.1:0
Jan 21 12:38:57.709 [notice] OR listener listening on port 15220.
Jan 21 12:38:57.709 [notice] Opened OR listener on 127.0.0.1:15220
Jan 21 12:38:57.709 [notice] Opening OR listener on [::1]:0
Jan 21 12:38:57.709 [notice] OR listener listening on port 36901.
Jan 21 12:38:57.709 [notice] Opened OR listener on [::1]:36901

This was introduced by commit 27c868eff19dbcc208f6db66ec3e2de2104fa754 and occurs in 0.3.5.1-alpha.

[kjak]

PR: ​https://github.com/torproject/tor/pull/663

This PR includes the fix and a changes file. make check-changes does not complain.

[teor]

Let's review this in 0.4.1, without discussing it at the team meeting.

[catalyst]

Looks good to me! I did a quick manual test with ControlPort auto and confirmed that the log message read as expected with a non-zero port number.

Note for whoever merges this: the pull request is based on master, but looks like it cherry-picks cleanly to 0.3.5.

[dgoulet]

Replying to catalyst:

Note for whoever merges this: the pull request is based on master, but looks like it cherry-picks cleanly to 0.3.5.

Not sure this qualifies for a backport as the log are at notice level. You think it should?

[teor]

In general, we are backporting ux issues to 0.3.5, including logging issues.
This issue is a minor source code change that is very low risk, so I will backport it to 0.3.5.

Please cherry-pick to maint-0.3.5 before merging to 0.4.0 and master.
Then I will backport it some time after the mainline CI passes.

[teor]

Actually, I will do the cherry-pick, because our draft merge policy says:

 * For patch quality: We should make sure that every commit to Tor is
    reviewed and double-checked for conformance.

…

4. What patches can be merged without review?

The following items do not need a reviewer or a separate merger:

Putting out releases:
   * Creating the ChangeLog and ReleaseNotes
   * Bumping the version number
   * Creating a signed tag

Typo fixes in comments.

But it seems redundant to require someone to do the cherry-pick, someone else to review it, and then a third person to merge. We could probably get away with 2 people.

[teor]

The 0.3.5 cherry-pick is:
​https://github.com/torproject/tor/pull/745

[catalyst]

Replying to teor:

The 0.3.5 cherry-pick is:
​https://github.com/torproject/tor/pull/745

Thanks! Code looks good. Quick manual test works as expected.

[nickm]

LGTM; merging to 0.4.0 and marking for backport.

[teor]

These tickets are low-risk changes that were marked for backport after the release of 0.4.0.2-alpha. I want them to get testing in at least one alpha release, so I'll look at them after 0.4.0.4-alpha is released.