[1806] in BarnOwl Developers
Re: Review of kcr/zsig branch.
daemon@ATHENA.MIT.EDU (Karl Ramm)
Thu Oct 29 18:22:25 2009
Resent-From: nelhage@mit.edu
Resent-To: barnowl-dev-mtg@charon.mit.edu
X-Original-To: nelhage@lunatique.mit.edu
From: Karl Ramm <kcr@1ts.org>
To: Nelson Elhage <nelhage@MIT.EDU>
Cc: barnowl-dev@MIT.EDU
Date: Sat, 17 Oct 2009 18:01:59 -0400
In-Reply-To: <20091017204203.GB21998@mit.edu> (Nelson Elhage's message of
"Sat, 17 Oct 2009 16:42:03 -0400")
New zsig branch posted.
Nelson Elhage <nelhage@MIT.EDU> writes:
> 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.
Gone.
>
> + 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.
Ooh, yeah, something about perl makes me forget that things can fail.
Fixed.
> 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.
I think not zeroing the zsig list if the file goes away (i.e. in all cases
where the open fails) is a reasonable behavior.
> - Nelson
