Various coding style fixes and cleanup based on a much-appreciated

code audit by Simon Cozens.  I didn't take all of his advise, and he
shouldn't be blamed for any remaining issues.

8 files changed, 107 insertions, 103 deletions

diff --git a/NEWS b/NEWS
index fa5e527..177c591 100644
--- a/NEWS
+++ b/NEWS
@@ -16,6 +16,10 @@ wallet 0.4 (unreleased)
 
     Add a missing class mapping for the netdb ACL schema verifier.
 
+    Various coding style fixes and cleanup based on a much-appreciated
+    code audit by Simon Cozens.  I didn't take all of his advise, and he
+    shouldn't be blamed for any remaining issues.
+
 wallet 0.3 (2007-12-03)
 
     MySQL is now a supported database backend and the full test suite
diff --git a/perl/Wallet/ACL.pm b/perl/Wallet/ACL.pm
index 3745074..f9a163b 100644
--- a/perl/Wallet/ACL.pm
+++ b/perl/Wallet/ACL.pm
@@ -174,7 +174,7 @@ sub rename {
     my ($self, $name) = @_;
     if ($name =~ /^\d+\z/) {
         $self->error ("ACL name may not be all numbers");
-        return undef;
+        return;
     }
     eval {
         my $sql = 'update acls set ac_name = ? where ac_id = ?';
@@ -184,7 +184,7 @@ sub rename {
     if ($@) {
         $self->error ("cannot rename ACL $self->{id} to $name: $@");
         $self->{dbh}->rollback;
-        return undef;
+        return;
     }
     $self->{name} = $name;
     return 1;
@@ -208,7 +208,7 @@ sub destroy {
     if ($@) {
         $self->error ("cannot destroy ACL $self->{id}: $@");
         $self->{dbh}->rollback;
-        return undef;
+        return;
     }
     return 1;
 }
@@ -223,7 +223,7 @@ sub add {
     $time ||= time;
     unless ($self->scheme_mapping ($scheme)) {
         $self->error ("unknown ACL scheme $scheme");
-        return undef;
+        return;
     }
     eval {
         my $sql = 'insert into acl_entries (ae_id, ae_scheme, ae_identifier)
@@ -235,7 +235,7 @@ sub add {
     if ($@) {
         $self->error ("cannot add $scheme:$identifier to $self->{id}: $@");
         $self->{dbh}->rollback;
-        return undef;
+        return;
     }
     return 1;
 }
@@ -264,7 +264,7 @@ sub remove {
         my $entry = "$scheme:$identifier";
         $self->error ("cannot remove $entry from $self->{id}: $@");
         $self->{dbh}->rollback;
-        return undef;
+        return;
     }
     return 1;
 }
@@ -308,7 +308,7 @@ sub show {
     my ($self) = @_;
     my @entries = $self->list;
     if (not @entries and $self->error) {
-        return undef;
+        return;
     }
     my $name = $self->name;
     my $id = $self->id;
@@ -344,7 +344,7 @@ sub history {
     if ($@) {
         $self->error ("cannot read history for $self->{id}: $@");
         $self->{dbh}->rollback;
-        return undef;
+        return;
     }
     return $output;
 }
@@ -392,7 +392,7 @@ sub check {
     my ($self, $principal) = @_;
     unless ($principal) {
         $self->error ('no principal specified');
-        return undef;
+        return;
     }
     my @entries = $self->list;
     return undef if (not @entries and $self->error);
diff --git a/perl/Wallet/ACL/Krb5.pm b/perl/Wallet/ACL/Krb5.pm
index 8c0610b..f94475f 100644
--- a/perl/Wallet/ACL/Krb5.pm
+++ b/perl/Wallet/ACL/Krb5.pm
@@ -35,11 +35,11 @@ sub check {
     my ($self, $principal, $acl) = @_;
     unless ($principal) {
         $self->error ('no principal specified');
-        return undef;
+        return;
     }
     unless ($acl) {
         $self->error ('malformed krb5 ACL');
-        return undef;
+        return;
     }
     return ($principal eq $acl) ? 1 : 0;
 }
diff --git a/perl/Wallet/ACL/NetDB.pm b/perl/Wallet/ACL/NetDB.pm
index 6437ebc..0d12703 100644
--- a/perl/Wallet/ACL/NetDB.pm
+++ b/perl/Wallet/ACL/NetDB.pm
@@ -65,11 +65,11 @@ sub check {
     my ($self, $principal, $acl) = @_;
     unless ($principal) {
         $self->error ('no principal specified');
-        return undef;
+        return;
     }
     unless ($acl) {
         $self->error ('malformed netdb ACL');
-        return undef;
+        return;
     }
     my $remctl = $self->{remctl};
     if ($Wallet::Config::NETDB_REALM) {
@@ -77,7 +77,7 @@ sub check {
     }
     unless ($remctl->command ('netdb', 'node-roles', $principal, $acl)) {
         $self->error ('cannot check NetDB ACL: ' . $remctl->error);
-        return undef;
+        return;
     }
     my ($roles, $output, $status, $error);
     do {
@@ -90,12 +90,12 @@ sub check {
             }
         } elsif ($output->type eq 'error') {
             $self->error ('cannot check NetDB ACL: ' . $output->data);
-            return undef;
+            return;
         } elsif ($output->type eq 'status') {
             $status = $output->status;
         } else {
             $self->error ('malformed NetDB remctl token: ' . $output->type);
-            return undef;
+            return;
         }
     } while ($output->type eq 'output');
     if ($status == 0) {
@@ -115,7 +115,7 @@ sub check {
         } else {
             $self->error ("error checking NetDB ACL");
         }
-        return undef;
+        return;
     }
 }
 
diff --git a/perl/Wallet/ACL/NetDB/Root.pm b/perl/Wallet/ACL/NetDB/Root.pm
index 5400d99..6ce7c88 100644
--- a/perl/Wallet/ACL/NetDB/Root.pm
+++ b/perl/Wallet/ACL/NetDB/Root.pm
@@ -37,7 +37,7 @@ sub check {
     my ($self, $principal, $acl) = @_;
     unless ($principal) {
         $self->error ('no principal specified');
-        return undef;
+        return;
     }
     unless ($principal =~ s%^([^/\@]+)/root(\@|\z)%$1$2%) {
         return 0;
diff --git a/perl/Wallet/Object/Base.pm b/perl/Wallet/Object/Base.pm
index ff6f008..e09c477 100644
--- a/perl/Wallet/Object/Base.pm
+++ b/perl/Wallet/Object/Base.pm
@@ -125,7 +125,7 @@ sub log_action {
     my ($self, $action, $user, $host, $time) = @_;
     unless ($action =~ /^(get|store)\z/) {
         $self->error ("invalid history action $action");
-        return undef;
+        return;
     }
 
     # We have two traces to record, one in the object_history table and one in
@@ -155,7 +155,7 @@ sub log_action {
         my $id = $self->{type} . ':' . $self->{name};
         $self->error ("cannot update history for $id: $@");
         $self->{dbh}->rollback;
-        return undef;
+        return;
     }
     return 1;
 }
@@ -268,7 +268,7 @@ sub acl {
         eval { $acl = Wallet::ACL->new ($id, $self->{dbh}) };
         if ($@) {
             $self->error ($@);
-            return undef;
+            return;
         }
         return $self->_set_internal ($attr, $acl->id, $user, $host, $time);
     } elsif (defined $id) {
@@ -325,7 +325,7 @@ sub owner {
         eval { $acl = Wallet::ACL->new ($owner, $self->{dbh}) };
         if ($@) {
             $self->error ($@);
-            return undef;
+            return;
         }
         return $self->_set_internal ('owner', $acl->id, $user, $host, $time);
     } elsif (defined $owner) {
@@ -386,7 +386,7 @@ sub flag_clear {
     if ($@) {
         $self->error ("cannot clear flag $flag on ${type}:${name}: $@");
         $dbh->rollback;
-        return undef;
+        return;
     }
     return 1;
 }
@@ -443,7 +443,7 @@ sub flag_set {
     if ($@) {
         $self->error ("cannot set flag $flag on ${type}:${name}: $@");
         $dbh->rollback;
-        return undef;
+        return;
     }
     return 1;
 }
@@ -503,7 +503,7 @@ sub history {
         my $id = $self->{type} . ':' . $self->{name};
         $self->error ("cannot read history for $id: $@");
         $self->{dbh}->rollback;
-        return undef;
+        return;
     }
     return $output;
 }
@@ -564,7 +564,7 @@ sub show {
     if ($@) {
         $self->error ("cannot retrieve data for ${type}:${name}: $@");
         $self->{dbh}->rollback;
-        return undef;
+        return;
     }
     my $output = '';
     my @acls;
@@ -575,14 +575,14 @@ sub show {
         if ($attrs[$i][0] eq 'ob_created_by') {
             my @flags = $self->flag_list;
             if (not @flags and $self->error) {
-                return undef;
+                return;
             }
             if (@flags) {
                 $output .= sprintf ("%15s: %s\n", 'Flags', "@flags");
             }
             my $attr_output = $self->attr_show;
             if (not defined $attr_output) {
-                return undef;
+                return;
             }
             $output .= $attr_output;
         }
@@ -632,7 +632,7 @@ sub destroy {
     if ($@) {
         $self->error ("cannot destroy ${type}:${name}: $@");
         $self->{dbh}->rollback;
-        return undef;
+        return;
     }
     return 1;
 }
@@ -654,7 +654,7 @@ Wallet::Object::Base - Generic parent class for wallet objects
     @ISA = qw(Wallet::Object::Base);
     sub get {
         my ($self, $user, $host, $time) = @_;
-        $self->log_action ('get', $user, $host, $time) or return undef;
+        $self->log_action ('get', $user, $host, $time) or return;
         return "Some secure data";
     }
 
diff --git a/perl/Wallet/Object/Keytab.pm b/perl/Wallet/Object/Keytab.pm
index 7aeb5da..ed998aa 100644
--- a/perl/Wallet/Object/Keytab.pm
+++ b/perl/Wallet/Object/Keytab.pm
@@ -95,7 +95,7 @@ sub kadmin_exists {
     }
     my $output = $self->kadmin ("getprinc $principal");
     if ($output =~ /^get_principal: /) {
-        return undef;
+        return;
     } else {
         return 1;
     }
@@ -129,7 +129,7 @@ sub kadmin_ktadd {
     my ($self, $principal, $file, @enctypes) = @_;
     unless ($self->valid_principal ($principal)) {
         $self->error ("invalid principal name: $principal");
-        return undef;
+        return;
     }
     if ($Wallet::Config::KEYTAB_REALM) {
         $principal .= '@' . $Wallet::Config::KEYTAB_REALM;
@@ -142,10 +142,10 @@ sub kadmin_ktadd {
     my $output = eval { $self->kadmin ("$command $principal") };
     if ($@) {
         $self->error ($@);
-        return undef;
+        return;
     } elsif ($output =~ /^(?:kadmin|ktadd): (.*)/m) {
         $self->error ("error creating keytab for $principal: $1");
-        return undef;
+        return;
     }
     return 1;
 }
@@ -157,12 +157,12 @@ sub kadmin_delprinc {
     my ($self, $principal) = @_;
     unless ($self->valid_principal ($principal)) {
         $self->error ("invalid principal name: $principal");
-        return undef;
+        return;
     }
     my $exists = eval { $self->kadmin_exists ($principal) };
     if ($@) {
         $self->error ($@);
-        return undef;
+        return;
     } elsif (not $exists) {
         return 1;
     }
@@ -172,10 +172,10 @@ sub kadmin_delprinc {
     my $output = eval { $self->kadmin ("delprinc -force $principal") };
     if ($@) {
         $self->error ($@);
-        return undef;
+        return;
     } elsif ($output =~ /^delete_principal: (.*)/m) {
         $self->error ("error deleting $principal: $1");
-        return undef;
+        return;
     }
     return 1;
 }
@@ -195,7 +195,7 @@ sub kaserver_name {
     $k5 =~ s/\@.*//;
     my @parts = split ('/', $k5);
     if (@parts > 2) {
-        return undef;
+        return;
     } elsif (@parts == 2 and $host{$parts[0]}) {
         $parts[1] =~ s/\..*//;
         $parts[0] = 'rcmd' if $parts[0] eq 'host';
@@ -216,12 +216,12 @@ sub kaserver_kasetkey {
     my $kasetkey = $Wallet::Config::KEYTAB_AFS_KASETKEY;
     unless ($kasetkey and $admin and $admin_srvtab) {
         $self->error ('kaserver synchronization not configured');
-        return undef;
+        return;
     }
     my $pid = open (KASETKEY, '-|');
     if (not defined $pid) {
         $self->error ("cannot fork: $!");
-        return undef;
+        return;
     } elsif ($pid == 0) {
         # Don't use die here; it will get trapped as an exception.  Also be
         # careful about our database handles.  (We still lose if there's some
@@ -244,7 +244,7 @@ sub kaserver_kasetkey {
             $output =~ s/\n/, /g;
             $output = ': ' . $output if $output;
             $self->error ("cannot synchronize key with kaserver$output");
-            return undef;
+            return;
         }
     }
     return 1;
@@ -262,12 +262,12 @@ sub kaserver_srvtab {
     eval { require Authen::Krb5 };
     if ($@) {
         $self->error ("kaserver synchronization support not available: $@");
-        return undef;
+        return;
     }
     eval { Authen::Krb5::init_context() };
     if ($@ and not $@ =~ /^Authen::Krb5 already initialized/) {
         $self->error ('Kerberos initialization failed');
-        return undef;
+        return;
     }
     undef $@;
 
@@ -279,17 +279,17 @@ sub kaserver_srvtab {
     unless (defined $princ) {
         my $error = Authen::Krb5::error();
         $self->error ("cannot parse $k5: $error");
-        return undef;
+        return;
     }
     my $key = Authen::Krb5::kt_read_service_key ($keytab, $princ, 0, 1);
     unless (defined $key) {
         my $error = Authen::Krb5::error();
         $self->error ("cannot find des-cbc-crc key in $keytab: $error");
-        return undef;
+        return;
     }
     unless (open (SRVTAB, '>', $srvtab)) {
         $self->error ("cannot create $srvtab: $!");
-        return undef;
+        return;
     }
 
     # srvtab format is nul-terminated name, nul-terminated instance,
@@ -305,7 +305,7 @@ sub kaserver_srvtab {
     unless (close SRVTAB) {
         unlink $srvtab;
         $self->error ("cannot write to $srvtab: $!");
-        return undef;
+        return;
     }
     return 1;
 }
@@ -321,15 +321,15 @@ sub kaserver_sync {
     my $k4 = $self->kaserver_name ($principal);
     if (not defined $k4) {
         $self->error ("cannot convert $principal to Kerberos v4");
-        return undef;
+        return;
     }
     my $srvtab = $Wallet::Config::KEYTAB_TMP . "/srvtab.$$";
     unless ($self->kaserver_srvtab ($keytab, $principal, $srvtab, $k4)) {
-        return undef;
+        return;
     }
     unless ($self->kaserver_kasetkey ('-c', $srvtab, '-s', $k4)) {
         unlink $srvtab;
-        return undef;
+        return;
     }
     unlink $srvtab;
     return 1;
@@ -343,7 +343,7 @@ sub kaserver_destroy {
     my $k4 = $self->kaserver_name ($principal);
     if (not defined $k4) {
         $self->error ("cannot convert $principal to Kerberos v4");
-        return undef;
+        return;
     }
     return $self->kaserver_kasetkey ('-D', $k4);
 }
@@ -371,7 +371,7 @@ sub kaserver_set {
     if ($@) {
         $self->error ($@);
         $self->{dbh}->rollback;
-        return undef;
+        return;
     }
     return 1;
 }
@@ -398,7 +398,7 @@ sub kaserver_clear {
     if ($@) {
         $self->error ($@);
         $self->{dbh}->rollback;
-        return undef;
+        return;
     }
     return 1;
 }
@@ -455,7 +455,7 @@ sub enctypes_set {
     if ($@) {
         $self->error ($@);
         $self->{dbh}->rollback;
-        return undef;
+        return;
     }
     return 1;
 }
@@ -500,12 +500,12 @@ sub keytab_retrieve {
     my $host = $Wallet::Config::KEYTAB_REMCTL_HOST;
     unless ($host and $Wallet::Config::KEYTAB_REMCTL_CACHE) {
         $self->error ('keytab unchanging support not configured');
-        return undef;
+        return;
     }
     eval { require Net::Remctl };
     if ($@) {
         $self->error ("keytab unchanging support not available: $@");
-        return undef;
+        return;
     }
     if ($Wallet::Config::KEYTAB_REALM) {
         $keytab .= '@' . $Wallet::Config::KEYTAB_REALM;
@@ -517,13 +517,13 @@ sub keytab_retrieve {
     my $result = Net::Remctl::remctl ($host, $port, $principal, @command);
     if ($result->error) {
         $self->error ("cannot retrieve keytab for $keytab: ", $result->error);
-        return undef;
+        return;
     } elsif ($result->status != 0) {
         my $error = $result->stderr;
         $error =~ s/\s+$//;
         $error =~ s/\n/ /g;
         $self->error ("cannot retrieve keytab for $keytab: $error");
-        return undef;
+        return;
     } else {
         return $result->stdout;
     }
@@ -591,13 +591,13 @@ sub attr_show {
     my $output = '';
     my @targets = $self->attr ('sync');
     if (not @targets and $self->error) {
-        return undef;
+        return;
     } elsif (@targets) {
         $output .= sprintf ("%15s: %s\n", 'Synced with', "@targets");
     }
     my @enctypes = $self->attr ('enctypes');
     if (not @enctypes and $self->error) {
-        return undef;
+        return;
     } elsif (@enctypes) {
         $output .= sprintf ("%15s: %s\n", 'Enctypes', $enctypes[0]);
         shift @enctypes;
@@ -631,7 +631,7 @@ sub destroy {
     my @sync = $self->attr ('sync');
     if (grep { $_ eq 'kaserver' } @sync) {
         unless ($self->kaserver_destroy ($self->{name})) {
-            return undef;
+            return;
         }
     }
     eval {
@@ -644,7 +644,7 @@ sub destroy {
     if ($@) {
         $self->error ($@);
         $self->{dbh}->rollback;
-        return undef;
+        return;
     }
     return undef if not $self->kadmin_delprinc ($self->{name});
     return $self->SUPER::destroy ($user, $host, $time);
@@ -669,7 +669,7 @@ sub get {
     }
     unless (defined ($Wallet::Config::KEYTAB_TMP)) {
         $self->error ('KEYTAB_TMP configuration variable not set');
-        return undef;
+        return;
     }
     my $file = $Wallet::Config::KEYTAB_TMP . "/keytab.$$";
     unlink $file;
@@ -679,7 +679,7 @@ sub get {
     unless (open (KEYTAB, '<', $file)) {
         my $princ = $self->{name};
         $self->error ("error opening keytab for principal $princ: $!");
-        return undef;
+        return;
     }
     local $/;
     undef $!;
@@ -688,14 +688,14 @@ sub get {
         my $princ = $self->{name};
         $self->error ("error reading keytab for principal $princ: $!");
         unlink $file;
-        return undef;
+        return;
     }
     close KEYTAB;
     my @sync = $self->attr ('sync');
     if (grep { $_ eq 'kaserver' } @sync) {
         unless ($self->kaserver_sync ($self->{name}, $file)) {
             unlink $file;
-            return undef;
+            return;
         }
     } elsif ($Wallet::Config::KEYTAB_AFS_DESTROY) {
         $self->kaserver_destroy ($self->{name});
diff --git a/perl/Wallet/Server.pm b/perl/Wallet/Server.pm
index 1e86057..1fa7e4a 100644
--- a/perl/Wallet/Server.pm
+++ b/perl/Wallet/Server.pm
@@ -228,7 +228,7 @@ sub create {
     my $class = $self->type_mapping ($type);
     unless ($class) {
         $self->error ("unknown object type $type");
-        return undef;
+        return;
     }
     my $dbh = $self->{dbh};
     my $user = $self->{user};
@@ -258,12 +258,12 @@ sub retrieve {
     my $class = $self->type_mapping ($type);
     unless ($class) {
         $self->error ("unknown object type $type");
-        return undef;
+        return;
     }
     my $object = eval { $class->new ($type, $name, $self->{dbh}) };
     if ($@) {
         $self->error ($@);
-        return undef;
+        return;
     } else {
         return $object;
     }
@@ -294,7 +294,7 @@ sub acl_check {
     my ($self, $object, $action) = @_;
     unless ($action =~ /^(get|store|show|destroy|flags|setattr|getattr)\z/) {
         $self->error ("unknown action $action");
-        return undef;
+        return;
     }
     if ($action ne 'get' and $action ne 'store') {
         return 1 if $self->{admin}->check ($self->{user});
@@ -312,22 +312,22 @@ sub acl_check {
     }
     unless (defined $id) {
         $self->object_error ($object, $action);
-        return undef;
+        return;
     }
     my $acl = eval { Wallet::ACL->new ($id, $self->{dbh}) };
     if ($@) {
         $self->error ($@);
-        return undef;
+        return;
     }
     my $status = $acl->check ($self->{user});
     if ($status == 1) {
         return 1;
     } elsif (not defined $status) {
         $self->error ($acl->error);
-        return undef;
+        return;
     } else {
         $self->object_error ($object, $action);
-        return undef;
+        return;
     }
 }
 
@@ -339,7 +339,7 @@ sub acl {
     return undef unless defined $object;
     unless ($self->{admin}->check ($self->{user})) {
         $self->object_error ($object, 'ACL');
-        return undef;
+        return;
     }
     my $result;
     if (defined $id) {
@@ -386,7 +386,7 @@ sub expires {
     return undef unless defined $object;
     unless ($self->{admin}->check ($self->{user})) {
         $self->object_error ($object, 'expires');
-        return undef;
+        return;
     }
     my $result;
     if (defined $expires) {
@@ -408,7 +408,7 @@ sub owner {
     return undef unless defined $object;
     unless ($self->{admin}->check ($self->{user})) {
         $self->object_error ($object, 'owner');
-        return undef;
+        return;
     }
     my $result;
     if (defined $owner) {
@@ -458,7 +458,7 @@ sub store {
     return undef unless $self->acl_check ($object, 'store');
     if (not defined ($data)) {
         $self->{error} = "no data supplied to store";
-        return undef;
+        return;
     }
     my $result = $object->store ($data, $self->{user}, $self->{host});
     $self->error ($object->error) unless defined $result;
@@ -541,7 +541,7 @@ sub acl_create {
     my ($self, $name) = @_;
     unless ($self->{admin}->check ($self->{user})) {
         $self->error ("$self->{user} not authorized to create ACL");
-        return undef;
+        return;
     }
     my $dbh = $self->{dbh};
     my $user = $self->{user};
@@ -549,7 +549,7 @@ sub acl_create {
     my $acl = eval { Wallet::ACL->create ($name, $dbh, $user, $host) };
     if ($@) {
         $self->error ($@);
-        return undef;
+        return;
     } else {
         return 1;
     }
@@ -575,17 +575,17 @@ sub acl_history {
     my ($self, $id) = @_;
     unless ($self->{admin}->check ($self->{user})) {
         $self->acl_error ($id, 'history');
-        return undef;
+        return;
     }
     my $acl = eval { Wallet::ACL->new ($id, $self->{dbh}) };
     if ($@) {
         $self->error ($@);
-        return undef;
+        return;
     }
     my $result = $acl->history;
     if (not defined $result) {
         $self->error ($acl->error);
-        return undef;
+        return;
     }
     return $result;
 }
@@ -595,17 +595,17 @@ sub acl_show {
     my ($self, $id) = @_;
     unless ($self->{admin}->check ($self->{user})) {
         $self->acl_error ($id, 'show');
-        return undef;
+        return;
     }
     my $acl = eval { Wallet::ACL->new ($id, $self->{dbh}) };
     if ($@) {
         $self->error ($@);
-        return undef;
+        return;
     }
     my $result = $acl->show;
     if (not defined $result) {
         $self->error ($acl->error);
-        return undef;
+        return;
     }
     return $result;
 }
@@ -616,20 +616,20 @@ sub acl_rename {
     my ($self, $id, $name) = @_;
     unless ($self->{admin}->check ($self->{user})) {
         $self->acl_error ($id, 'rename');
-        return undef;
+        return;
     }
     my $acl = eval { Wallet::ACL->new ($id, $self->{dbh}) };
     if ($@) {
         $self->error ($@);
-        return undef;
+        return;
     }
     if ($acl->name eq 'ADMIN') {
         $self->error ('cannot rename the ADMIN ACL');
-        return undef;
+        return;
     }
     unless ($acl->rename ($name)) {
         $self->error ($acl->error);
-        return undef;
+        return;
     }
     return 1;
 }
@@ -640,20 +640,20 @@ sub acl_destroy {
     my ($self, $id) = @_;
     unless ($self->{admin}->check ($self->{user})) {
         $self->acl_error ($id, 'destroy');
-        return undef;
+        return;
     }
     my $acl = eval { Wallet::ACL->new ($id, $self->{dbh}) };
     if ($@) {
         $self->error ($@);
-        return undef;
+        return;
     }
     if ($acl->name eq 'ADMIN') {
         $self->error ('cannot destroy the ADMIN ACL');
-        return undef;
+        return;
     }
     unless ($acl->destroy ($self->{user}, $self->{host})) {
         $self->error ($acl->error);
-        return undef;
+        return;
     }
     return 1;
 }
@@ -664,16 +664,16 @@ sub acl_add {
     my ($self, $id, $scheme, $identifier) = @_;
     unless ($self->{admin}->check ($self->{user})) {
         $self->acl_error ($id, 'add');
-        return undef;
+        return;
     }
     my $acl = eval { Wallet::ACL->new ($id, $self->{dbh}) };
     if ($@) {
         $self->error ($@);
-        return undef;
+        return;
     }
     unless ($acl->add ($scheme, $identifier, $self->{user}, $self->{host})) {
         $self->error ($acl->error);
-        return undef;
+        return;
     }
     return 1;
 }
@@ -684,28 +684,28 @@ sub acl_remove {
     my ($self, $id, $scheme, $identifier) = @_;
     unless ($self->{admin}->check ($self->{user})) {
         $self->acl_error ($id, 'remove');
-        return undef;
+        return;
     }
     my $acl = eval { Wallet::ACL->new ($id, $self->{dbh}) };
     if ($@) {
         $self->error ($@);
-        return undef;
+        return;
     }
     if ($acl->name eq 'ADMIN') {
         my @e = $acl->list;
         if (not @e and $acl->error) {
             $self->error ($acl->error);
-            return undef;
+            return;
         } elsif (@e == 1 && $e[0][0] eq $scheme && $e[0][1] eq $identifier) {
             $self->error ('cannot remove last ADMIN ACL entry');
-            return undef;
+            return;
         }
     }
     my $user = $self->{user};
     my $host = $self->{host};
     unless ($acl->remove ($scheme, $identifier, $user, $host)) {
         $self->error ($acl->error);
-        return undef;
+        return;
     }
     return 1;
 }