Re: [Tails-dev] tordate issue in Tails 0.14~rc1

Supprimer ce message

Répondre à ce message
Auteur: intrigeri
Date:  
À: The Tails public development discussion list
Sujet: Re: [Tails-dev] tordate issue in Tails 0.14~rc1
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.