[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #5018 [Tor]: don't start ClientTransportPlugin proxies until we have a bridge that wants them



#5018: don't start ClientTransportPlugin proxies until we have a bridge that wants
them
------------------------+--------------------------------
     Reporter:  arma    |      Owner:
         Type:  defect  |     Status:  needs_revision
     Priority:  normal  |  Milestone:  Tor: 0.2.5.x-final
    Component:  Tor     |    Version:
   Resolution:          |   Keywords:  pt tor-bridge
Actual Points:          |  Parent ID:
       Points:          |
------------------------+--------------------------------
Changes (by asn):

 * status:  needs_review => needs_revision


Comment:

 Replying to [comment:11 arma]:
 > Initial review:
 >
 > - The function comment for parse_client_transport_line() needs to change
 too, yes?
 >

 True. Need to fix.

 > - I think your transport_is_needed() is simpler as just:
 > {{{
 > /** Return True if we have a bridge that uses a transport with name
 >  *  <b>transport_name</b>. */
 > int
 > transport_is_needed(const char *transport_name)
 > {
 >   if (!bridge_list)
 >     return 0;
 >
 >   SMARTLIST_FOREACH_BEGIN(bridge_list, const bridge_info_t *, bridge) {
 >     if (bridge->transport_name &&
 >         !strcmp(bridge->transport_name, transport_name))
 >       return 1;
 >   } SMARTLIST_FOREACH_END(bridge);
 >
 >   return 0;
 > }
 > }}}
 > and that makes me wonder about whether the comparison should be case-
 sensitive or insensitive. (How is this handled elsewhere in the code?
 Looks like case-sensitive?)

 Also true. My original function was indeed very suboptimal.

 > - This code has no plans for shutting down transports that no longer
 have bridges asking for them, yes? I think that's ok for a first go, but
 want to point it out in case people want to work on that later.

 Right. This should probably be a new trac ticket.

 > - Man -- my code above looks almost exactly like the
 transport_get_by_name() function, doesn't it?

 Yep, even though these two functions cannot be merged in an obvious
 manner, we should look into this.

 Moving this to `needs_revision`.

-- 
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/5018#comment:13>
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