hi,
anonym wrote (18 Oct 2012 11:18:43 GMT) :
> Implemented in bugfix/tordate_vs_tor_0.2.3.x
Looks good.
I'm worried about the reliance on exact log messages wording,
but well...
Here follows some nitpicking on implementation details.
I'm in doubt with the "grep -m 1" on Tor log file: don't we want the
*last* occurrence of e.g. "certificate lifetime", rather than the
first one, in case there would be several? This may very well be
a rare edge case, or even impossible, but still, if we indeed want the
last occurrence, I'd rather see this expressed explicitly in code.
Same for:
> grep -q "\[warn\] Certificate \(not yet valid\|already expired\)." \
> /var/log/tor/log
About "grep -m 1 SIMPLIFIED_PATTERN | sed ...": I think this can be
done in a more robust way with sed only, which would also be a way to
get the last occurrence if that's what we want.
Please use long options (e.g. --max-count, --only-matching, etc.):
more readable, easier to review and maintain.
About "grep | tail -n1": I think "tac | grep --max-count 1",
or sed alone, may be a bit cheaper. Not that we care much.
> Grep Tor's log instead of GETINFO the control port.
> Through extensive testing it seems that opening a controller and/or
> issuing "GETINFO status/bootstrap-phase" during early bootstrap gets
> Tor stuck at "Bootstrapped 5%".
Argh :( Upstream bug report?
Weird fact: Vidalia seems to be able to do the same without triggering
this bug.
Nitpicking (about the commented-out tor_control_getinfo()): according
to the Tor control protocol spec, even if Tor MAY accept LF alone,
"Controllers SHOULD always send CRLF".
To end with, the design doc might need updating.
Nice job!
> (merged it into experimental).
It seems you forgot to push.
