[tor-bugs] #6236 [Core Tor/Tor]: Remove duplicate code between parse_{c, s}method_line

Tor Bug Tracker & Wiki blackhole at torproject.org
Sat Feb 24 12:44:50 UTC 2018


#6236: Remove duplicate code between parse_{c,s}method_line
-------------------------------------------------+-------------------------
 Reporter:  asn                                  |          Owner:  (none)
     Type:  task                                 |         Status:  new
 Priority:  Medium                               |      Milestone:  Tor:
                                                 |  unspecified
Component:  Core Tor/Tor                         |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  tor-client easy refactor duplicate-  |  Actual Points:
  code                                           |
Parent ID:                                       |         Points:  1
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------

Comment (by asn):

 Replying to [comment:13 fristonio]:
 > Sorry asn I was a bit busy with my exams.
 >
 > I have created a patch for this
 https://github.com/fristonio/tor/tree/ticket-6236
 > Please have a look and suggest any changes if required.

 Hello! Thanks for the code! Concept looks pretty good!

 Some simplification suggestions:
 a) You don't really need to compare `proto_method` with `PROTO_CMETHOD`
 since you control those two things yourself anyway. Instead of
 `proto_method`, I'd pass an `is_smethod` boolean variable to the helper
 function which would specify whether it's an SMETHOD or CMETHOD line. That
 way you don't need to do all these strcmps either, and you can just do `if
 (is_smethod) { }`.
 b) You don't really need this `proxy_manager` thing. You just use it in a
 log statement, and there you can do:
 {{{
     log_warn(LD_CONFIG, "%s managed proxy sent us a %s line "
              "with too few arguments.", is_smethod ? "Server" : "Client",
              proto_method);
 }}}
 c) You don't need the `else` clause in the final logging if block. Since
 you have already validated on top that `proto_method` is either CMETHOD or
 SMETHOD.
 d) You don't need to pass `SMETHOD_MIN_ARGS_COUNT` as a function argument.
 Instead you can set `min_args_count` inside the helper function based on
 `is_smethod`.

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


More information about the tor-bugs mailing list