[1802] in BarnOwl Developers
Review of kcr/zsig branch.
daemon@ATHENA.MIT.EDU (Nelson Elhage)
Thu Oct 29 18:22:21 2009
Resent-From: nelhage@mit.edu
Resent-To: barnowl-dev-mtg@charon.mit.edu
X-Original-To: nelhage@lunatique.mit.edu
Date: Sat, 17 Oct 2009 16:42:03 -0400
From: Nelson Elhage <nelhage@MIT.EDU>
To: kcr@mit.edu
Cc: barnowl-dev@mit.edu
Overall, looks about right. A few comments:
+srand; # sigh
This is unnecessary. Quoting `perldoc -f srand`:
If srand() is not called explicitly, it is called implicitly at
the first use of the "rand" operator. However, this was not the
case in versions of Perl before 5.004, so if your script will
run under older Perl versions, it should call "srand".
No one runs barnowl on perl 5.004 (Hell, probably no one runs 5.004,
at all, and I bet barnowl doesn't even work against it), so you can
just trust perl to seed itself as needed.
+ my $old_mtime = $loaded_mtime;
+ my ($dev, $ino, $mode, $nlink, $uid, $gid, $rdev, $size,
+ $atime, $loaded_mtime, $ctime, $blksize, $blocks) = stat($random_zsig_file);
+ return $old_mtime != $loaded_mtime;
From `perldoc -f stat`:
[stat] returns a null list if the stat fails.
If stat fails, because, e.g. we don't have tokens, this will result in
'$loaded_mtime' getting assigned 'undef', which will cause a "Use of
uninitialized value $mtime in numeric ne (!=)" warning on the '!='. It
will also cause != to return 1, which will make us try to reload the
zsigs. Assuming we don't have tokens, this will cause the open() to
fail and we will fall back to the default zsig, which I don't think is
intended.
It's possible that if the open() fails, we should check $! to see why
it failed, and keep the previous set if zsigs (if any) if it failed
for a reason other than ENOENT. That would result in something closer
to the behavior I think you're going for if you lose tokens. It might
result in confusing behavior in some other case, but I can't think of
any, right now.
- Nelson
