On Mon, Dec 29, 2014 at 3:33 PM, Tom van der Woerdt info@tvdw.eu wrote:
Sounds good!
I spent some time writing a patch that removes v1 of the link protocol from both the server and client, and so far it seems to work nicely: the code compiles nicely, all test cases pass, and the resulting binary has relayed a few gigabytes of data without any problems.
As I didn't really have a place to put the branch, I uploaded it to Github: https://github.com/TvdW/tor/commits/master
It's a rather large patch, though not as large as the patch that will remove v2 of the protocol. However, before I write that one, can someone please check whether my patch is sane and I'm not violating any standards or policies?
Hi, Tom!
Sorry about the delay; this has been a busy time, between CCC drama and the new year.
A few procedural suggestions:
* Maybe call branches like this something other than "master"? It's a good idea to have a separate topic branch for each patch series you write, so that the upstream project can merge them independently.
* It's a good idea to link to branches like this from the appropriate tickets on the bugtracker, so that we can't lose track of them.
* Every Tuesday on the #tor-dev IRC channel on OFTC, at 1800 UTC, it's patch workshop time, where a bunch of people get together to review one another's patches. Maybe you'd like to stop by? :)
And, some fast notes on the patch itself:
* Actually, it's not that long. "git log -p -b" only says it's 607 lines, which is comparatively easy to review.
* Some of the stuff can't actually get removed yet by the terms of what we've said we're doing. The V2_CIPHER_LIST, for example, is used to detect Tor versions up through 0.2.3.17-beta. We've only mentioned dropping support for 0.2.2 and earlier, right?. Similarly with the tor_tls_session_secret_cb logic.
Now, maybe we _should_ drop support for versions before 0.2.3.17-beta as well. If so, we can rip out even more code. (And that might be a good idea.) What do people on the list think?
I'll try to have a closer look over the weekend. Thanks for the code!
yrs,