Browse Source

[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>
tags/v1.20.1
Michael Brown 11 years ago
parent
commit
72fb55e437
3 changed files with 8 additions and 47 deletions
  1. 2
    14
      src/core/settings.c
  2. 4
    17
      src/crypto/clientcert.c
  3. 2
    16
      src/crypto/rootcert.c

+ 2
- 14
src/core/settings.c View File

722
  *
722
  *
723
  * The caller is responsible for eventually freeing the allocated
723
  * The caller is responsible for eventually freeing the allocated
724
  * buffer.
724
  * buffer.
725
- *
726
- * To allow the caller to distinguish between a non-existent setting
727
- * and an error in allocating memory for the copy, this function will
728
- * return success (and a NULL buffer pointer) for a non-existent
729
- * setting.
730
  */
725
  */
731
 int fetch_setting_copy ( struct settings *settings, struct setting *setting,
726
 int fetch_setting_copy ( struct settings *settings, struct setting *setting,
732
 			 void **data ) {
727
 			 void **data ) {
736
 	/* Avoid returning uninitialised data on error */
731
 	/* Avoid returning uninitialised data on error */
737
 	*data = NULL;
732
 	*data = NULL;
738
 
733
 
739
-	/* Fetch setting length, and return success if non-existent */
734
+	/* Check existence, and fetch setting length */
740
 	len = fetch_setting_len ( settings, setting );
735
 	len = fetch_setting_len ( settings, setting );
741
 	if ( len < 0 )
736
 	if ( len < 0 )
742
-		return 0;
737
+		return len;
743
 
738
 
744
 	/* Allocate buffer */
739
 	/* Allocate buffer */
745
 	*data = malloc ( len );
740
 	*data = malloc ( len );
1055
 		goto err_fetch_copy;
1050
 		goto err_fetch_copy;
1056
 	}
1051
 	}
1057
 
1052
 
1058
-	/* Return error if setting does not exist */
1059
-	if ( ! raw ) {
1060
-		ret = -ENOENT;
1061
-		goto err_exists;
1062
-	}
1063
-
1064
 	/* Sanity check */
1053
 	/* Sanity check */
1065
 	assert ( setting->type != NULL );
1054
 	assert ( setting->type != NULL );
1066
 	assert ( setting->type->format != NULL );
1055
 	assert ( setting->type->format != NULL );
1071
 
1060
 
1072
  err_format:
1061
  err_format:
1073
 	free ( raw );
1062
 	free ( raw );
1074
- err_exists:
1075
  err_fetch_copy:
1063
  err_fetch_copy:
1076
 	return ret;
1064
 	return ret;
1077
 }
1065
 }

+ 4
- 17
src/crypto/clientcert.c View File

116
 	static void *cert = NULL;
116
 	static void *cert = NULL;
117
 	static void *key = NULL;
117
 	static void *key = NULL;
118
 	int len;
118
 	int len;
119
-	int rc;
120
 
119
 
121
 	/* Allow client certificate to be overridden only if
120
 	/* Allow client certificate to be overridden only if
122
 	 * not explicitly specified at build time.
121
 	 * not explicitly specified at build time.
129
 
128
 
130
 		/* Fetch new client certificate, if any */
129
 		/* Fetch new client certificate, if any */
131
 		free ( cert );
130
 		free ( cert );
132
-		len = fetch_setting_copy ( NULL, &cert_setting, &cert );
133
-		if ( len < 0 ) {
134
-			rc = len;
135
-			DBGC ( &client_certificate, "CLIENTCERT cannot fetch "
136
-			       "client certificate: %s\n", strerror ( rc ) );
137
-			return rc;
138
-		}
139
-		if ( cert ) {
131
+		if ( ( len = fetch_setting_copy ( NULL, &cert_setting,
132
+						  &cert ) ) >= 0 ) {
140
 			client_certificate.data = cert;
133
 			client_certificate.data = cert;
141
 			client_certificate.len = len;
134
 			client_certificate.len = len;
142
 		}
135
 		}
147
 
140
 
148
 		/* Fetch new client private key, if any */
141
 		/* Fetch new client private key, if any */
149
 		free ( key );
142
 		free ( key );
150
-		len = fetch_setting_copy ( NULL, &privkey_setting, &key );
151
-		if ( len < 0 ) {
152
-			rc = len;
153
-			DBGC ( &client_certificate, "CLIENTCERT cannot fetch "
154
-			       "client private key: %s\n", strerror ( rc ) );
155
-			return rc;
156
-		}
157
-		if ( key ) {
143
+		if ( ( len = fetch_setting_copy ( NULL, &privkey_setting,
144
+						  &key ) ) >= 0 ) {
158
 			client_private_key.data = key;
145
 			client_private_key.data = key;
159
 			client_private_key.len = len;
146
 			client_private_key.len = len;
160
 		}
147
 		}

+ 2
- 16
src/crypto/rootcert.c View File

91
 static void rootcert_init ( void ) {
91
 static void rootcert_init ( void ) {
92
 	void *external = NULL;
92
 	void *external = NULL;
93
 	int len;
93
 	int len;
94
-	int rc;
95
 
94
 
96
 	/* Allow trusted root certificates to be overridden only if
95
 	/* Allow trusted root certificates to be overridden only if
97
 	 * not explicitly specified at build time.
96
 	 * not explicitly specified at build time.
101
 		/* Fetch copy of "trust" setting, if it exists.  This
100
 		/* Fetch copy of "trust" setting, if it exists.  This
102
 		 * memory will never be freed.
101
 		 * memory will never be freed.
103
 		 */
102
 		 */
104
-		len = fetch_setting_copy ( NULL, &trust_setting, &external );
105
-		if ( len < 0 ) {
106
-			rc = len;
107
-			DBGC ( &root_certificates, "ROOTCERT cannot fetch "
108
-			       "trusted root certificate fingerprints: %s\n",
109
-			       strerror ( rc ) );
110
-			/* No way to prevent startup; fail safe by
111
-			 * trusting no certificates.
112
-			 */
113
-			root_certificates.count = 0;
114
-			return;
115
-		}
116
-
117
-		/* Use certificates from "trust" setting, if present */
118
-		if ( external ) {
103
+		if ( ( len = fetch_setting_copy ( NULL, &trust_setting,
104
+						  &external ) ) >= 0 ) {
119
 			root_certificates.fingerprints = external;
105
 			root_certificates.fingerprints = external;
120
 			root_certificates.count = ( len / FINGERPRINT_LEN );
106
 			root_certificates.count = ( len / FINGERPRINT_LEN );
121
 		}
107
 		}

Loading…
Cancel
Save