diff options
author | Russ Allbery <rra@stanford.edu> | 2007-12-05 22:55:45 +0000 |
---|---|---|
committer | Russ Allbery <rra@stanford.edu> | 2007-12-05 22:55:45 +0000 |
commit | f8c47d1f3cf5d4e8aa64c39120731ca9054dc786 (patch) | |
tree | 479e5d45b62e70ddfe8f15d2efc0a05cc60ca158 /perl | |
parent | 5ad61b6ecd14adffa013ef5b0b8c2b8da8cca03b (diff) |
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.
Diffstat (limited to 'perl')
-rw-r--r-- | perl/Wallet/ACL.pm | 18 | ||||
-rw-r--r-- | perl/Wallet/ACL/Krb5.pm | 4 | ||||
-rw-r--r-- | perl/Wallet/ACL/NetDB.pm | 12 | ||||
-rw-r--r-- | perl/Wallet/ACL/NetDB/Root.pm | 2 | ||||
-rw-r--r-- | perl/Wallet/Object/Base.pm | 24 | ||||
-rw-r--r-- | perl/Wallet/Object/Keytab.pm | 74 | ||||
-rw-r--r-- | perl/Wallet/Server.pm | 72 |
7 files changed, 103 insertions, 103 deletions
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; } |