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

Re: [tor-bugs] #7952 [Tor]: Control port method to get the exit policy

#7952: Control port method to get the exit policy
------------------------------------------+---------------------------------
 Reporter:  atagar                        |          Owner:                    
     Type:  enhancement                   |         Status:  needs_revision    
 Priority:  normal                        |      Milestone:  Tor: 0.2.5.x-final
Component:  Tor                           |        Version:                    
 Keywords:  tor-relay easy small-feature  |         Parent:                    
   Points:                                |   Actualpoints:                    
------------------------------------------+---------------------------------
Changes (by nickm):

  * status:  needs_review => needs_revision


Comment:

 Thanks!  To answer your question, POLICY_BUF_LEN was meant to hold a
 single IPv4 policy item, not the entire exit policy.

 Here are some more comments:
    * router_get_my_routerinfo() is allowed to return NULL, so
 dereferencing its result without checking it is a bad idea.  Similarly,
 I'd be more comfortable with a check that router->exit_policy is non-NULL,
 even though that strictly speaking shouldn't be needed.
    * Instead of accumulating these entries in a buffer, it's probably
 better to put them all in a smartlist, and then use
 smartlist_join_strings() to concatenate them.  (That way we wouldn't need
 to worry about pointer math, running out of space, or the much-deprecated
 strcat().)
    * The preferred pattern to iterate over a smartlist is
 SMARTLIST_FOREACH_BEGIN/SMARTLIST_FOREACH_END.
    * I like the idea of breaking router_dump_exit_policy_to_string() into
 a new function.
    * Calling smartlist_add_asprintf(chunks, "%s\n", exit_policy);
 introduces a memory leak, since exit_policy is now never freed.
    * Perhaps there should be a variant GETINFO that *does* return IPv6
 policy items?  If so, the sensible way to implement it would probably be
 to add an IPv4/IPv6 flag as an argument to
 router_dump_exit_policy_to_string().
    * router_dump_exit_policy_to_string() definitely needs a unit test.
 (Let me know if it's not clear how to write one.)

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