[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