[tor-bugs] #5040 [Tor]: Make public bridges add obfsproxy stats to their extra-info descriptors

[Tor Bug Tracker & Wiki]

#5040: Make public bridges add obfsproxy stats to their extra-info descriptors
----------------------------------------------------------------+-----------
 Reporter:  karsten                                             |          Owner:  asn               
     Type:  enhancement                                         |         Status:  needs_review      
 Priority:  normal                                              |      Milestone:  Tor: 0.2.5.x-final
Component:  Tor                                                 |        Version:                    
 Keywords:  pt tor-bridge flashproxy SponsorL SponsorF20131101  |         Parent:                    
   Points:                                                      |   Actualpoints:                    
----------------------------------------------------------------+-----------
Changes (by asn):

  * status:  needs_revision => needs_review


Comment:

 Replying to [comment:19 nickm]:
 > Okay. This review is based on "git log -p --reverse
 26e5db3118e7b46deb28^..d355e8372602d4f2826e9df2afa5a1b536f436d0"
 >
 > General:
 >    * This needs an accompanying torspec patch.
 >

 For the torspec branch see tickets #7751 and #8045.

 > 26e5db3118e7b46deb287c4ff9460d28c83c01b1
 >    * Use tor_memdup_nul_terminate() or whatever it's called for
 connection_or_handle_cmd_transport.

 I couldn't find tor_memdup_nul_terminate() or any other related function.
 Do you maybe remember the price name?

 >    * The ext_or_transport field needs to document what the format is for
 the "name of the pt obfuscating this connection".
 >    * What happens if an IP connects twice with different transports?
 What *should* happen?  (Does the unit test check that?)
 >
 > bca5b2ce343d1a34ac7168fb4d4a74334e367244:
 >    * The smartlist_string_isin() check will make the algorithm O(n)^2.
 Why not use "if (val == 1)"?
 >    * Does transport_counts hold an signed intptr_t, or an unsigned
 uintptr_t, or a value in the range of 'unsigned'? You're using it both
 ways.
 >    * You don't need to use i as an index; just use
 "transport_name_sl_idx", the implicit index that you get for free.
 >    * Actually, why not forget the business with "," vs "" in the loop,
 and instead pass "," as the joining string to smartlist_join_strings?
 >
 > Looks okay otherwise.

 Refetch my `bug5040` branch for fixes on the rest of your comments.

 BTW, do you think that `clientmap_entry_hash()` can be more elegant? My
 new changes look a bit bulky there.

-- 
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/5040#comment:20>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online