[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #5040 [Tor]: Make public bridges add obfsproxy stats to their extra-info descriptors
#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
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs