From 8b9daeaad45a5181356d76fa9dfde6db25296d69 Mon Sep 17 00:00:00 2001 From: Russ Allbery Date: Tue, 28 Aug 2007 21:12:15 +0000 Subject: Throw exceptions for failure of object new() and create() functions rather than returning undef so that we can save the error message. --- docs/design-api | 18 +++++++++++------- perl/Wallet/Object/Base.pm | 23 +++++++++++------------ perl/Wallet/Object/Keytab.pm | 11 ++++++----- 3 files changed, 28 insertions(+), 24 deletions(-) diff --git a/docs/design-api b/docs/design-api index 80e6988..7657b27 100644 --- a/docs/design-api +++ b/docs/design-api @@ -17,11 +17,11 @@ Object API new(NAME, TYPE, DBH) - Creates a new object with the given object name and type based on - data already in the database. Takes a database handle, which should - be stored with the object and used for any further operations. This - method should inherit from the generic Wallet::Object object, which - implements the following methods: + Creates a new object with the given object name and type based on data + already in the database. Takes a database handle, which should be + stored with the object and used for any further operations. This + method should inherit from the generic Wallet::Object::Base object, + which implements the following methods: new(NAME, DBH) create(NAME, DBH) @@ -34,14 +34,18 @@ Object API error() that manipulate the basic object data. Generally all this function - needs to do is call the parent new() constructor. + needs to do is call the parent new() constructor. If the object + couldn't be found, throws an exception. (Just returning undef would + provide no way of communicating the error message.) create(NAME, TYPE, DBH, PRINCIPAL, HOSTNAME [, DATETIME]) Like new(), but instead creates a new entry in the database with the given name. As with new(), the generic function will normally do all of the work. Takes some additional information to put into the - created fields in the database. + created fields in the database. If the object already exists or + creating it fails, throws an exception. (Just returning undef would + provide no way of communicating the error message.) destroy(PRINCIPAL, HOSTNAME [, DATETIME]) diff --git a/perl/Wallet/Object/Base.pm b/perl/Wallet/Object/Base.pm index a3c9b3d..d50e513 100644 --- a/perl/Wallet/Object/Base.pm +++ b/perl/Wallet/Object/Base.pm @@ -40,7 +40,7 @@ sub new { $dbh->{PrintError} = 0; my $sql = 'select ob_name from objects where ob_name = ? and ob_type = ?'; my $data = $dbh->selectrow_array ($sql, undef, $name, $type); - return undef unless ($data and $data eq $name); + die "cannot find ${type}:${name}\n" unless ($data and $data eq $name); my $self = { dbh => $dbh, name => $name, @@ -71,7 +71,7 @@ sub create { }; if ($@) { $dbh->rollback; - return undef; + die "cannot create object ${type}:${name}: $@\n"; } my $self = { dbh => $dbh, @@ -385,9 +385,9 @@ the Wallet::Object::Type->new syntax). Creates a new object with the given object name and type, based on data already in the database. This method will only succeed if an object of the given NAME and TYPE is already present in the wallet database. If no such -object exits, returns undef. Otherwise, returns an object blessed into the -class used for the new() call (so subclasses can leave this method alone and -not override it). +object exits, throws an exception. Otherwise, returns an object blessed +into the class used for the new() call (so subclasses can leave this method +alone and not override it). Takes a database handle, which is stored in the object and used for any further operations. This database handle is taken over by the wallet system @@ -397,13 +397,12 @@ object for its own needs. =item create(NAME, TYPE, DBH, PRINCIPAL, HOSTNAME [, DATETIME]) Similar to new() but instead creates a new entry in the database. This -method will fail (returning undef) if an entry for that name and type -already exists in the database or if creating the database record fails. -Otherwise, a new database entry will be created with that name and type, no -owner, no ACLs, no expiration, no flags, and with created by, from, and on -set to the PRINCIPAL, HOSTNAME, and DATETIME parameters. If DATETIME isn't -given, the current time is used. The database handle is treated as with -new(). +method will throw an exception if an entry for that name and type already +exists in the database or if creating the database record fails. Otherwise, +a new database entry will be created with that name and type, no owner, no +ACLs, no expiration, no flags, and with created by, from, and on set to the +PRINCIPAL, HOSTNAME, and DATETIME parameters. If DATETIME isn't given, the +current time is used. The database handle is treated as with new(). =back diff --git a/perl/Wallet/Object/Keytab.pm b/perl/Wallet/Object/Keytab.pm index b10c67a..b386fae 100644 --- a/perl/Wallet/Object/Keytab.pm +++ b/perl/Wallet/Object/Keytab.pm @@ -80,17 +80,18 @@ sub _kadmin_exists { } } -# Create a principal in Kerberos. Return true if successful, false otherwise. +# Create a principal in Kerberos. Since this is only called by create, it +# throws an exception on failure rather than setting the error and returning +# undef. sub _kadmin_addprinc { my ($self, $principal) = @_; unless ($self->_valid_principal ($principal)) { - $self->{error} = "invalid principal name: $principal"; - return undef; + die "invalid principal name $principal\n"; } my $flags = $Wallet::Config::KEYTAB_FLAGS; my $output = $self->_kadmin ("addprinc -randkey $flags $principal"); if ($output =~ /^add_principal: (.*)/m) { - return undef; + die "error adding principal $principal: $!\n"; } return 1; } @@ -144,7 +145,7 @@ sub create { if ($name !~ /\@/ && $Wallet::Config::KEYTAB_REALM) { $name .= '@' . $Wallet::Config::KEYTAB_REALM; } - return undef if not $class->_kadmin_addprinc ($name); + $class->_kadmin_addprinc ($name); return $class->SUPER::create ($name, $type, $dbh, $creator, $host, $time); } -- cgit v1.2.3