You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@spamassassin.apache.org by Daniel Quinlan <qu...@pathname.com> on 2005/04/27 23:42:17 UTC
quick audit of die()
I've been going through the code a bit reviewing log messages, now die()
since it's logged too and everything appears to be properly formatted,
etc. However, I'm a bit concerned that we use die() in places where
errors are quite recoverable. Some of them I'm fixing right now, but
I'd like some feedback/help with these. If you know what to change,
preferably the help rather than the feedback. ;-)
Thanks.
Daniel
------------------------------------------------------------------------
DBM: should these really be fatal errors?
BayesStore/DBM.pm-1384- # make temporary copy since old dbm and new dbm may have same name
BayesStore/DBM.pm:1385: opendir(DIR, $dir) || die "bayes: can't opendir $dir: $!";
BayesStore/DBM.pm-1386- my @files = grep { /^bayes_(?:seen|toks)(?:\.\w+)?$/ } readdir(DIR);
BayesStore/DBM.pm-1389- {
BayesStore/DBM.pm:1390: die "bayes: unable to find bayes_toks and bayes_seen, stopping\n";
BayesStore/DBM.pm-1391- }
BayesStore/DBM.pm-1397- my $dst = "$dir/old_$_";
BayesStore/DBM.pm:1398: copy($src, $dst) || die "bayes: can't copy $src to $dst: $!\n";
BayesStore/DBM.pm-1399- }
------------------------------------------------------------------------
Locker: are these unrecoverable too?
Locker/UnixNFSSafe.pm-74- umask $umask; # just in case
Locker/UnixNFSSafe.pm:75: die "locker: safe_lock: $$ cannot create tmp lockfile $lock_tmp for $lock_file: $!\n";
Locker/UnixNFSSafe.pm-76- }
Locker/UnixNFSSafe.pm-152- if (!defined $ourtmp_ctime) {
Locker/UnixNFSSafe.pm:153: die "locker: safe_unlock: stat failed on $lock_tmp";
Locker/UnixNFSSafe.pm-154- }
Locker/Flock.pm-60- umask $umask; # just in case
Locker/Flock.pm:61: die "locker: safe_lock: $$ cannot create lockfile $lock_file: $!\n";
Locker/Flock.pm-62- }
Locker/Flock.pm-72- eval {
Locker/Flock.pm:73: local $SIG{ALRM} = sub { die "alarm\n" };
Locker/Flock.pm-74- dbg("locker: safe_lock: $$ trying to get lock on $path with $max_retries timeout");
Locker/Flock.pm-99-
Locker/Flock.pm:100: $unalarmed or alarm $oldalarm; # if we die'd above, need to reset here
Locker/Flock.pm-101- if ($err) {
Locker/Flock.pm-104- } else {
Locker/Flock.pm:105: die "locker: safe_lock: $$ $err";
Locker/Flock.pm-106- }
------------------------------------------------------------------------
SQL: I suppose these are needed, but just asking...
Conf/SQL.pm-99- # make sure we can see croak messages from DBI
Conf/SQL.pm:100: local $SIG{'__DIE__'} = sub { die "$_[0]"; };
Conf/SQL.pm-101- require DBI;
Conf/SQL.pm-165- else {
Conf/SQL.pm:166: die "config: SQL error: $sql\n".$sth->errstr."\n";
Conf/SQL.pm-167- }
Conf/SQL.pm-169- else {
Conf/SQL.pm:170: die "config: SQL error: " . $dbh->errstr . "\n";
Conf/SQL.pm-171- }
Conf/SQL.pm-174- else {
Conf/SQL.pm:175: die "config: SQL error: " . DBI->errstr . "\n";
Conf/SQL.pm-176- }
------------------------------------------------------------------------
DBBasedAddrList: Do these need to be fatal (PersistentAddrList was
similar, but I left it out of this summary)?
DBBasedAddrList.pm-62- if (!$dbm_module) {
DBBasedAddrList.pm:63: die "auto-whitelist: cannot find a usable DB package from auto_whitelist_db_modules: " .
DBBasedAddrList.pm-64- $main->{conf}->{auto_whitelist_db_modules}."\n";
DBBasedAddrList.pm-102- }
DBBasedAddrList.pm:103: die "auto-whitelist: cannot open auto_whitelist_path $path: $!\n";
DBBasedAddrList.pm-104-}
------------------------------------------------------------------------
Util: I really doubt all of these need to be die()
Util.pm-828- if (!$tmpdir) {
Util.pm:829: die "util: cannot find a temporary directory, set TMP or TMPDIR in environment";
Util.pm-830- }
Util.pm-1114- if ($< != $touid) {
Util.pm:1115: die "util: setuid $< to $touid failed!";
Util.pm-1116- }
Util.pm-1144- if (!defined $pid) {
Util.pm:1145: die "util: cannot fork: $!";
Util.pm-1146- }
Util.pm-1173- }
Util.pm:1174: open STDIN, "<$stdinfile" or die "util: cannot open $stdinfile: $!";
Util.pm-1175-
Util.pm-1178- # will be the lowest unused fd number, which should be 0.
Util.pm:1179: # so die with a useful error if this somehow isn't the case.
Util.pm-1180- if (fileno(STDIN) != 0) {
Util.pm:1181: die "util: setuid: oops: fileno(STDIN) [".fileno(STDIN)."] != 0";
Util.pm-1182- }
Util.pm-1190- # if (fileno(STDOUT) != 1) {
Util.pm:1191: # die "setuid: oops: fileno(STDOUT) [".fileno(STDOUT)."] != 1";
Util.pm-1192- # }
Util.pm-1201- }
Util.pm:1202: open STDERR, ">&STDOUT" or die "util: dup STDOUT failed: $!";
Util.pm-1203-
Util.pm-1205- if (fileno(STDERR) != 2) {
Util.pm:1206: die "util: setuid: oops: fileno(STDERR) [".fileno(STDERR)."] != 2";
Util.pm-1207- }
Util.pm-1210- exec @cmdline;
Util.pm:1211: die "util: exec failed: $!";
Util.pm-1212-}
------------------------------------------------------------------------
Reporter: same question...
Reporter.pm-100- if (WIFEXITED ($exitstatus) && (WEXITSTATUS ($exitstatus))) {
Reporter.pm:101: die "reporter: exited with non-zero exit code " . WEXITSTATUS($exitstatus) . "\n";
Reporter.pm-102- }
Reporter.pm-104- if (WIFSIGNALED ($exitstatus)) {
Reporter.pm:105: die "reporter: exited due to signal " . WTERMSIG($exitstatus) . "\n";
Reporter.pm-106- }
--
Daniel Quinlan
http://www.pathname.com/~quinlan/
Re: quick audit of die()
Posted by Daniel Quinlan <qu...@pathname.com>.
Bueller?
Bueller?
--
Daniel Quinlan
http://www.pathname.com/~quinlan/
Re: quick audit of die()
Posted by Daniel Quinlan <qu...@pathname.com>.
Justin/Theo, do these changes look good? Maybe I'm being paranoid
as the die() statements seem hard to reach, but I'm paranoid. ;-)
Index: lib/Mail/SpamAssassin/Message/Node.pm
===================================================================
--- lib/Mail/SpamAssassin/Message/Node.pm (revision 165034)
+++ lib/Mail/SpamAssassin/Message/Node.pm (working copy)
@@ -485,7 +485,8 @@
return Mail::SpamAssassin::Util::qp_decode($data);
}
else {
- die "message: unknown encoding type '$cte' in RFC2047 header";
+ warn "message: unknown encoding type '$cte' in RFC2047 header";
+ return $data;
}
}
Index: lib/Mail/SpamAssassin/Conf/Parser.pm
===================================================================
--- lib/Mail/SpamAssassin/Conf/Parser.pm (revision 165034)
+++ lib/Mail/SpamAssassin/Conf/Parser.pm (working copy)
@@ -272,6 +272,8 @@
$self->{currentfile}.": if ".$cond->{conditional}."\n";
}
else {
+ # die seems a bit excessive here, but this shouldn't be possible
+ # so I suppose it's okay.
die "config: unknown 'if' type: ".$cond->{type}."\n";
}
@@ -366,7 +368,9 @@
}
if (!$cmd->{code}) {
- $self->setup_default_code_cb ($cmd);
+ if (! $self->setup_default_code_cb($cmd)) {
+ goto failed_line;
+ }
}
my $ret = &{$cmd->{code}} ($conf, $cmd->{setting}, $value, $line);
@@ -560,8 +564,10 @@
$cmd->{code} = \&set_template_append;
}
else {
- die "config: unknown conf type $type!";
+ warn "config: unknown conf type $type!";
+ return 0;
}
+ return 1;
}
sub set_numeric_value {
--
Daniel Quinlan
http://www.pathname.com/~quinlan/