[Settings] copy_settings() should not fail if some settings are missing!

3 files changed, 71 insertions, 59 deletions

diff --git a/src/core/settings.c b/src/core/settings.c
index 49594dca..b78e05e2 100644
--- a/src/core/settings.c
+++ b/src/core/settings.c
@@ -499,40 +499,6 @@ unsigned long fetch_uintz_setting ( struct settings *settings,
 }
 
 /**
- * Copy setting
- *
- * @v dest		Destination settings block
- * @v dest_tag		Destination setting tag number
- * @v source		Source settings block
- * @v source_tag	Source setting tag number
- * @ret rc		Return status code
- */
-int copy_setting ( struct settings *dest, unsigned int dest_tag,
-		   struct settings *source, unsigned int source_tag ) {
-	int len;
-	int check_len;
-	int rc;
-
-	len = fetch_setting_len ( source, source_tag );
-	if ( len < 0 )
-		return len;
-
-	{
-		char buf[len];
-
-		check_len = fetch_setting ( source, source_tag, buf,
-					    sizeof ( buf ) );
-		assert ( check_len == len );
-
-		if ( ( rc = store_setting ( dest, dest_tag, buf,
-					    sizeof ( buf ) ) ) != 0 )
-			return rc;
-	}
-
-	return 0;
-}
-
-/**
  * Copy settings
  *
  * @v dest		Destination settings block
@@ -545,6 +511,8 @@ static int copy_encap_settings ( struct settings *dest,
 				 unsigned int encapsulator ) {
 	unsigned int subtag;
 	unsigned int tag;
+	int len;
+	int check_len;
 	int rc;
 
 	for ( subtag = DHCP_MIN_OPTION; subtag <= DHCP_MAX_OPTION; subtag++ ) {
@@ -552,16 +520,26 @@ static int copy_encap_settings ( struct settings *dest,
 		switch ( tag ) {
 		case DHCP_EB_ENCAP:
 		case DHCP_VENDOR_ENCAP:
-			/* Process encapsulated options field */
+			/* Process encapsulated settings */
 			if ( ( rc = copy_encap_settings ( dest, source,
 							  tag ) ) != 0 )
 				return rc;
 			break;
 		default:
-			/* Copy option to reassembled packet */
-			if ( ( rc = copy_setting ( dest, tag, source,
-						   tag ) ) != 0 )
-				return rc;
+			/* Copy setting, if present */
+			len = fetch_setting_len ( source, tag );
+			if ( len < 0 )
+				break;
+			{
+				char buf[len];
+
+				check_len = fetch_setting ( source, tag, buf,
+							    sizeof ( buf ) );
+				assert ( check_len == len );
+				if ( ( rc = store_setting ( dest, tag, buf,
+							    sizeof(buf) )) !=0)
+					return rc;
+			}
 			break;
 		}
 	}
diff --git a/src/include/gpxe/settings.h b/src/include/gpxe/settings.h
index 643bd05b..f32d3ec9 100644
--- a/src/include/gpxe/settings.h
+++ b/src/include/gpxe/settings.h
@@ -163,8 +163,6 @@ extern int store_setting ( struct settings *settings, unsigned int tag,
 			   const void *data, size_t len );
 extern int fetch_setting ( struct settings *settings, unsigned int tag,
 			   void *data, size_t len );
-extern int copy_setting ( struct settings *dest, unsigned int dest_tag,
-			  struct settings *source, unsigned int source_tag );
 extern int copy_settings ( struct settings *dest, struct settings *source );
 extern int fetch_setting_len ( struct settings *settings, unsigned int tag );
 extern int fetch_string_setting ( struct settings *settings, unsigned int tag,
diff --git a/src/net/udp/dhcp.c b/src/net/udp/dhcp.c
index 7f6722be..908e7b3f 100644
--- a/src/net/udp/dhcp.c
+++ b/src/net/udp/dhcp.c
@@ -255,20 +255,34 @@ static int create_dhcp_request ( struct dhcp_packet *dhcppkt,
 
 	/* Copy any required options from previous server repsonse */
 	if ( dhcpoffer ) {
-		if ( ( rc = copy_setting ( &dhcppkt->settings,
-					   DHCP_SERVER_IDENTIFIER,
-					   &dhcpoffer->settings,
-					   DHCP_SERVER_IDENTIFIER ) ) != 0 ) {
-			DBG ( "DHCP could not set server identifier "
-			      "option: %s\n", strerror ( rc ) );
+		struct in_addr server_id;
+		struct in_addr requested_ip;
+
+		if ( ( rc = fetch_ipv4_setting ( &dhcpoffer->settings,
+						 DHCP_SERVER_IDENTIFIER,
+						 &server_id ) ) < 0 ) {
+			DBG ( "DHCP offer missing server identifier\n" );
+			return -EINVAL;
+		}
+		if ( ( rc = fetch_ipv4_setting ( &dhcpoffer->settings,
+						 DHCP_EB_YIADDR,
+						 &requested_ip ) ) < 0 ) {
+			DBG ( "DHCP offer missing IP address\n" );
+			return -EINVAL;
+		}
+		if ( ( rc = store_setting ( &dhcppkt->settings,
+					    DHCP_SERVER_IDENTIFIER, &server_id,
+					    sizeof ( server_id ) ) ) != 0 ) {
+			DBG ( "DHCP could not set server identifier: %s\n ",
+			      strerror ( rc ) );
 			return rc;
 		}
-		if ( ( rc = copy_setting ( &dhcppkt->settings,
-					   DHCP_REQUESTED_ADDRESS,
-					   &dhcpoffer->settings,
-					   DHCP_EB_YIADDR ) ) != 0 ) {
-			DBG ( "DHCP could not set requested address "
-			      "option: %s\n", strerror ( rc ) );
+		if ( ( rc = store_setting ( &dhcppkt->settings,
+					    DHCP_REQUESTED_ADDRESS,
+					    &requested_ip,
+					    sizeof ( requested_ip ) ) ) != 0 ){
+			DBG ( "DHCP could not set requested address: %s\n",
+			      strerror ( rc ) );
 			return rc;
 		}
 	}
@@ -335,8 +349,16 @@ static int create_dhcp_request ( struct dhcp_packet *dhcppkt,
 int create_dhcpdiscover ( struct net_device *netdev,
 			  void *data, size_t max_len ) {
 	struct dhcp_packet dhcppkt;
+	int rc;
+
+	if ( ( rc = create_dhcp_request ( &dhcppkt, netdev, NULL, data,
+					  max_len ) ) != 0 ) {
+		DBG ( "Could not create DHCPDISCOVER: %s\n",
+		      strerror ( rc ) );
+		return rc;
+	}
 
-	return create_dhcp_request ( &dhcppkt, netdev, NULL, data, max_len );
+	return 0;
 }
 
 /**
@@ -356,18 +378,26 @@ int create_dhcpack ( struct net_device *netdev,
 
 	/* Create base DHCPACK packet */
 	if ( ( rc = create_dhcp_packet ( &dhcppkt, netdev, DHCPACK, NULL,
-					 data, max_len ) ) != 0 )
+					 data, max_len ) ) != 0 ) {
+		DBG ( "Could not create DHCPACK: %s\n", strerror ( rc ) );
 		return rc;
+	}
 
 	/* Merge in globally-scoped settings, then netdev-specific
 	 * settings.  Do it in this order so that netdev-specific
 	 * settings take precedence regardless of stated priorities.
 	 */
-	if ( ( rc = copy_settings ( &dhcppkt.settings, NULL ) ) != 0 )
+	if ( ( rc = copy_settings ( &dhcppkt.settings, NULL ) ) != 0 ) {
+		DBG ( "Could not set DHCPACK global settings: %s\n",
+		      strerror ( rc ) );
 		return rc;
+	}
 	if ( ( rc = copy_settings ( &dhcppkt.settings,
-				    netdev_settings ( netdev ) ) ) != 0 )
+				    netdev_settings ( netdev ) ) ) != 0 ) {
+		DBG ( "Could not set DHCPACK netdev settings: %s\n",
+		      strerror ( rc ) );
 		return rc;
+	}
 
 	return 0;
 }
@@ -399,12 +429,18 @@ int create_proxydhcpack ( struct net_device *netdev,
 
 	/* Create base DHCPACK packet */
 	if ( ( rc = create_dhcp_packet ( &dhcppkt, netdev, DHCPACK, NULL,
-					 data, max_len ) ) != 0 )
+					 data, max_len ) ) != 0 ) {
+		DBG ( "Could not create ProxyDHCPACK: %s\n",
+		      strerror ( rc ) );
 		return rc;
+	}
 
 	/* Merge in ProxyDHCP options */
-	if ( ( rc = copy_settings ( &dhcppkt.settings, settings ) ) != 0 )
+	if ( ( rc = copy_settings ( &dhcppkt.settings, settings ) ) != 0 ) {
+		DBG ( "Could not set ProxyDHCPACK settings: %s\n",
+		      strerror ( rc ) );
 		return rc;
+	}
 
 	return 0;
 }