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

Re: [tor-bugs] #1692 [Tor Relay]: No Events for SETCONF

#1692: No Events for SETCONF
-------------------------+--------------------------------------------------
 Reporter:  atagar       |       Owner:  atagar          
     Type:  enhancement  |      Status:  needs_review    
 Priority:  normal       |   Milestone:  Tor: unspecified
Component:  Tor Relay    |     Version:  Tor: unspecified
 Keywords:               |      Parent:                  
-------------------------+--------------------------------------------------

Comment(by Sebastian):

 If you can, referencing a git branch somewhere or attaching a patch
 generated with format-patch would be great.

 For review: Lacks a changes file, introduces a bunch of whitespace errors
 (run make check-spaces to catch those).

 AttributeList looks a little ill-defined, you should probably expand it a
 bit to explain what an attribute is (the word attribute doesn't appear
 anywhere else in control spec); make sure you also explain how attribute
 and value are separated, what happens when the value contains a space (how
 is it quoted?) etc. Just giving an example will not be enough, you need to
 be precise in the specification.

 you can replace the XXX with 0.2.3.1-alpha, if that version gets released
 without this patch we can still update it later.

 IIRC arma told me that we don't use // comments except to comment out
 code, so your comment should probably be wrapped in /* */.

 the if (old_options) check should probably have a comment explaining that
 old_options isn't set during first start of Tor, and that we can't
 usefully send an event that early anyways.

 You are also leaking memory: smartlist_join_strings(), smartlist_create()
 and tor_asprintf() allocate new memory that you have to free.

 Just a first glance, didn't have brains for a full review yet

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