[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #3656 [Tor Bridge]: Support spawning multiple protocols using the same managed proxy
#3656: Support spawning multiple protocols using the same managed proxy
------------------------+---------------------------------------------------
Reporter: asn | Owner: asn
Type: defect | Status: needs_review
Priority: normal | Milestone:
Component: Tor Bridge | Version:
Keywords: | Parent:
Points: | Actualpoints:
------------------------+---------------------------------------------------
Comment(by nickm):
Replying to [comment:6 asn]:
> === What I fixed: ===
Thanks! I'll take another pass through.
> I'm getting an: EOL@EOF:src/or/config.c:5995 warning in check-spaces. I
tried to remove the EOL with three different editors and I couldn't. I
gave up after a bit.
That warning means that there's supposed to be a blank line at the end of
the file.
> * Try this stuff under valgrind; there are enough new things allocated
in one place and freed in another that I'm a little unconfident that I
actually checked everything completely. (valgrind instructions in
doc/HACKING are a must)
>
> Did that a hundred times with different torrc etc. I plugged all
memleaks I managed to find, but some allocation are quite complex (and
there are many edge cases as well) and I'm still afraid I might have left
memleaks lying around...
Okay, we'll have to look more as we go.
The s
> * Please don't commit prints
>
> I always forget you are reviewing by reading diffs and I forget prints
inside. Sorry. I killed all stray printf()s in 2e73f9b3.
Even if I weren't reviewing by reading diffs, you still shouldn't commit
prints. :)
> * If I HUP tor to reload the configuration, do I wind up re-launching
all the proxies? I didn't see the code that prevents this from
happening.
>
> Alright, this was a hard one. I had to hunt for edge cases all around
the code and I didn't like it. I'm not very satisfied with the results but
I couldn't find a better way to do it. It should detect all torrc
modifications and react accordingly '''except''' when external and managed
proxies are mixed together. More on this later.
I'll check it out. Let me know if you want any advice here.
> === What I didn't fix: ===
> * would a comma-separated list of transports be easier than multiple
lines here? I'm not sure the behavior is intuitive as it is. Is this
how we designed it?
>
> It's true, comma-seperated transports will probably be more intuitive.
It's in my TODO list.
Okay. I'd rather not merge the interface only to change it later: if we
have one interface in a public release, we usually commit ourselves to
keeping that interface working indefinitely. So if we fix it before
merge, we don't have to keep the old thing working. I guess we could just
label the interface as "unstable, will change" for an alpha or two if we
really had to.
> == TODO ==
> We probably have to introduce a LOG_PT and put the transports.c etc.
logging in that domain.
LD_PT, you mean.
> We probably also have to introduce a unique identifier to every managed
proxy so that logs become a bit more meaningful.
Sounds smart.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/3656#comment:9>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs