On Mon, Dec 03, 2007 at 11:51:29PM +0100, Marcus Wolschon wrote: > > Dear list, > > I wrote an extensive patch to make tor support > * hidden services on ipv6-addresses > * binding for socks on an ipv6-address > * convert a lot of the internal data-structures for the comming full > ipv6-support. > > I wrote it to tor-volunteers@xxxxxxxxxxxxxx on friday but got no > answer thus I am posting them here. > > The code may not use the prefered naming-conventions or the > tor_address_t -data-structure (because that one uses socket_addr > and we only need the actual address-part) but I guess a refactoring > for it to meet the coding-guidelines is far less work then writing it > and getting it to work in the first place. Hi, Marcus! Thanks, and good job! The 0.2.0.x series is in feature-freeze now, but I'll save this patch to be sure to merge it once 0.2.1.x is branched. A few general points below in case you (or somebody else) wants to work on this patch some more. None of these are show-stoppers, but fixing them will make the patch better. - You're right that the preferred way to store addresses that could be either IPv4 or IPv6 is indeed with tor_addr_t. (Thanks for the reminder, BTW: I fixed tor_addr_t to be a tagged union of in_addr and in6_addr, not of sockaddr_in and sockaddr_in6.) - Generally speaking, we try to avoid duplicated "cut and paste" code; it makes it hard to be sure that we can update all instances of the original code path. (connection_or_init_conn_from_address6, for example, is almost exactly the same as the existing connection_init_conn_from_address.) - It's good to break up patches into functional parts, since shorter patches are way easier to review. For example, this patch includes lots of log cleanups that make the logs say "hidden service" instead of "service". That's a cleanup, and that can go into 0.2.0.x on its own, but it doesn't seem to be dependent much on the rest of the patch. Also, the different functional changes could probably be separated. Thanks again for your patch! I've wanted IPv6 support for a long time; it will be great to finally get it in. peace, -- Nick
Attachment:
pgp14HBpC4WrH.pgp
Description: PGP signature