Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[settings] Change "not-found" semantics of fetch_setting_copy()
fetch_settings_copy() currently returns success and a NULL data
pointer to indicate a non-existent setting.  This is intended to allow
the caller to differentiate between a non-existent setting and an
error in allocating memory for the copy of the setting.

The underlying settings blocks' fetch() methods provide no way to
perform an existence check separate from an attempt to fetch the
setting.  A "non-existent setting" therefore means simply a setting
for which an error was encountered when attempting to fetch from every
settings block within the subtree.

Since any underlying error within a settings block (e.g. a GuestRPC
failure when attempting to retrieve a VMware GuestInfo setting) will
produce the effect of a "non-existent setting", it seems somewhat
meaningless to give special treatment to memory allocation errors
within fetch_setting_copy().

Remove the special treatment and simplify the semantics of
fetch_setting_copy() by directly passing through any underlying error
(including non-existence) encountered while fetching the setting.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
  • Loading branch information
mcb30 committed Jul 19, 2013
1 parent 5ffcae6 commit 72fb55e
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 47 deletions.
16 changes: 2 additions & 14 deletions src/core/settings.c
Expand Up @@ -722,11 +722,6 @@ int fetch_setting_len ( struct settings *settings, struct setting *setting ) {
*
* The caller is responsible for eventually freeing the allocated
* buffer.
*
* To allow the caller to distinguish between a non-existent setting
* and an error in allocating memory for the copy, this function will
* return success (and a NULL buffer pointer) for a non-existent
* setting.
*/
int fetch_setting_copy ( struct settings *settings, struct setting *setting,
void **data ) {
Expand All @@ -736,10 +731,10 @@ int fetch_setting_copy ( struct settings *settings, struct setting *setting,
/* Avoid returning uninitialised data on error */
*data = NULL;

/* Fetch setting length, and return success if non-existent */
/* Check existence, and fetch setting length */
len = fetch_setting_len ( settings, setting );
if ( len < 0 )
return 0;
return len;

/* Allocate buffer */
*data = malloc ( len );
Expand Down Expand Up @@ -1055,12 +1050,6 @@ int fetchf_setting ( struct settings *settings, struct setting *setting,
goto err_fetch_copy;
}

/* Return error if setting does not exist */
if ( ! raw ) {
ret = -ENOENT;
goto err_exists;
}

/* Sanity check */
assert ( setting->type != NULL );
assert ( setting->type->format != NULL );
Expand All @@ -1071,7 +1060,6 @@ int fetchf_setting ( struct settings *settings, struct setting *setting,

err_format:
free ( raw );
err_exists:
err_fetch_copy:
return ret;
}
Expand Down
21 changes: 4 additions & 17 deletions src/crypto/clientcert.c
Expand Up @@ -116,7 +116,6 @@ static int clientcert_apply_settings ( void ) {
static void *cert = NULL;
static void *key = NULL;
int len;
int rc;

/* Allow client certificate to be overridden only if
* not explicitly specified at build time.
Expand All @@ -129,14 +128,8 @@ static int clientcert_apply_settings ( void ) {

/* Fetch new client certificate, if any */
free ( cert );
len = fetch_setting_copy ( NULL, &cert_setting, &cert );
if ( len < 0 ) {
rc = len;
DBGC ( &client_certificate, "CLIENTCERT cannot fetch "
"client certificate: %s\n", strerror ( rc ) );
return rc;
}
if ( cert ) {
if ( ( len = fetch_setting_copy ( NULL, &cert_setting,
&cert ) ) >= 0 ) {
client_certificate.data = cert;
client_certificate.len = len;
}
Expand All @@ -147,14 +140,8 @@ static int clientcert_apply_settings ( void ) {

/* Fetch new client private key, if any */
free ( key );
len = fetch_setting_copy ( NULL, &privkey_setting, &key );
if ( len < 0 ) {
rc = len;
DBGC ( &client_certificate, "CLIENTCERT cannot fetch "
"client private key: %s\n", strerror ( rc ) );
return rc;
}
if ( key ) {
if ( ( len = fetch_setting_copy ( NULL, &privkey_setting,
&key ) ) >= 0 ) {
client_private_key.data = key;
client_private_key.len = len;
}
Expand Down
18 changes: 2 additions & 16 deletions src/crypto/rootcert.c
Expand Up @@ -91,7 +91,6 @@ struct x509_root root_certificates = {
static void rootcert_init ( void ) {
void *external = NULL;
int len;
int rc;

/* Allow trusted root certificates to be overridden only if
* not explicitly specified at build time.
Expand All @@ -101,21 +100,8 @@ static void rootcert_init ( void ) {
/* Fetch copy of "trust" setting, if it exists. This
* memory will never be freed.
*/
len = fetch_setting_copy ( NULL, &trust_setting, &external );
if ( len < 0 ) {
rc = len;
DBGC ( &root_certificates, "ROOTCERT cannot fetch "
"trusted root certificate fingerprints: %s\n",
strerror ( rc ) );
/* No way to prevent startup; fail safe by
* trusting no certificates.
*/
root_certificates.count = 0;
return;
}

/* Use certificates from "trust" setting, if present */
if ( external ) {
if ( ( len = fetch_setting_copy ( NULL, &trust_setting,
&external ) ) >= 0 ) {
root_certificates.fingerprints = external;
root_certificates.count = ( len / FINGERPRINT_LEN );
}
Expand Down

0 comments on commit 72fb55e

Please sign in to comment.