Re: [Tails-dev] Made New Identity work again, please review

Delete this message

Reply to this message
Autore: intrigeri
Data:  
To: The Tails public development discussion list
Oggetto: Re: [Tails-dev] Made New Identity work again, please review
Hi,

(Note: it would be great not to break threading: anyone reading the
old thread, but not this one, will miss your reply etc. IIRC you read
the list via the archives, so I understand it's a bit of a pain, but
hopefully your MUA allows you to insert arbitrary References and
In-Reply-To headers.)

winterfairy@??? wrote (13 Feb 2014 12:20:36 GMT) :
> intrigeri wrote:
>> 1. I'm not entirely convinced by the idea of running
>>    tor-controlport-filter as the vidalia user. Granted, it's the
>>    shortest path, but it feels weird, and having a bit more privilege
>>    separation would be good.


> Fixed. Same branch.


Looks good to me.

>> 2. The use of string.find(expected text) seems a bit fragile.
>>    Couldn't this matching be done in a bit stricter way?


> I am not sure what you mean by stricter.
> I changed it to startswith() anyway, which is equivalent
> to what I did, but clearer.


> find() reports position of first occurence (-1 on error).
> I was testing that the position was == 0.


Oh, thanks for the explanation, and for making the code clearer.
Indeed the semantics of `find' were not obvious to me without reading
the doc.

>> 3. It might be overkill, and surely adds some code, but I would pass
>>    the port to listen on, control port socket path and authentication
>>    cookie path as command-line arguments (preferably named, not
>>    positional). Just to make it easier for others to reuse this piece
>>    of code, and increase collaboration between similar projects.


> A quick research showed up that this can probably be done
> with only a few lines of code if using the modules
> optparse or argparse.


> Python 2.6 (which Tails uses) only has optparse.
> In python 2.7 optparse is deprecated and replaced by argparse.
> This would mean writing one version for squeeze and one for wheezy.


> I don't think it is worth it currently.


Fair enough. Care to file a ticket, blocked by #6015, so that this is
not forgotten at post-1.1 time?

Merged again into experimental, pushed again to the main repo and
to Jenkins.

Cheers,
--
intrigeri
| GnuPG key @ https://gaffer.ptitcanardnoir.org/intrigeri/intrigeri.asc
| OTR fingerprint @ https://gaffer.ptitcanardnoir.org/intrigeri/otr.asc