[settings] Explicitly separate the concept of a completed fetched setting

The fetch_setting() family of functions may currently modify the
definition of the specified setting (e.g. to add missing type
information).  Clean up this interface by requiring callers to provide
an explicit buffer to contain the completed definition of the fetched
setting, if required.

Signed-off-by: Michael Brown <mcb30@ipxe.org>

3 files changed, 41 insertions, 45 deletions

diff --git a/src/hci/commands/nvo_cmd.c b/src/hci/commands/nvo_cmd.c
index 3fd684d0..e63dab08 100644
--- a/src/hci/commands/nvo_cmd.c
+++ b/src/hci/commands/nvo_cmd.c
@@ -58,8 +58,10 @@ static int show_exec ( int argc, char **argv ) {
 	struct show_options opts;
 	struct named_setting setting;
 	struct settings *origin;
+	struct setting fetched;
 	char name_buf[32];
 	char *value;
+	int len;
 	int rc;
 
 	/* Parse options */
@@ -71,19 +73,16 @@ static int show_exec ( int argc, char **argv ) {
 		goto err_parse_setting;
 
 	/* Fetch formatted setting value */
-	if ( ( rc = fetchf_setting_copy ( setting.settings, &setting.setting,
-					  &value ) ) < 0 ) {
+	if ( ( len = fetchf_setting_copy ( setting.settings, &setting.setting,
+					   &origin, &fetched, &value ) ) < 0 ) {
+		rc = len;
 		printf ( "Could not find \"%s\": %s\n",
 			 setting.setting.name, strerror ( rc ) );
 		goto err_fetchf;
 	}
 
-	/* Fetch origin and format fully-qualified name */
-	origin = fetch_setting_origin ( setting.settings, &setting.setting );
-	assert ( origin != NULL );
-	setting_name ( origin, &setting.setting, name_buf, sizeof ( name_buf ));
-
 	/* Print setting value */
+	setting_name ( origin, &fetched, name_buf, sizeof ( name_buf ) );
 	printf ( "%s = %s\n", name_buf, value );
 
 	/* Success */
@@ -234,7 +233,8 @@ static int read_value ( struct named_setting *setting, char **args __unused,
 	/* Read existing value, treating errors as equivalent to an
 	 * empty initial setting.
 	 */
-	fetchf_setting_copy ( setting->settings, &setting->setting, &existing );
+	fetchf_setting_copy ( setting->settings, &setting->setting,
+			      NULL, &setting->setting, &existing );
 
 	/* Read new value */
 	if ( ( rc = readline_history ( NULL, existing, NULL, value ) ) != 0 )
@@ -294,12 +294,11 @@ static int inc_exec ( int argc, char **argv ) {
 	     ( ( rc = parse_integer ( argv[ optind + 1 ], &increment ) ) != 0))
 		goto err_parse_increment;
 
-	/* Fetch existing setting value, if any, allowing for the fact
-	 * that numeric settings are big-endian and variable-length.
+	/* Read existing value, treating errors as equivalent to a
+	 * zero-valued :int32 initial setting.
 	 */
 	if ( ( rc = fetchn_setting ( setting.settings, &setting.setting,
-				     &value ) ) != 0 ) {
-		/* Treat as a non-existent :int32 setting with a zero value */
+				     NULL, &setting.setting, &value ) ) != 0 ) {
 		value = 0;
 		if ( ! setting.setting.type )
 			setting.setting.type = &setting_type_int32;
diff --git a/src/hci/commands/pci_cmd.c b/src/hci/commands/pci_cmd.c
index f2d69569..f5145fb3 100644
--- a/src/hci/commands/pci_cmd.c
+++ b/src/hci/commands/pci_cmd.c
@@ -68,8 +68,8 @@ static int pciscan_exec ( int argc, char **argv ) {
 		goto err_parse_setting;
 
 	/* Determine starting bus:dev.fn address */
-	if ( ( len = fetch_uint_setting ( setting.settings, &setting.setting,
-					  &prev ) ) < 0 ) {
+	if ( ( len = fetchn_setting ( setting.settings, &setting.setting,
+				      NULL, &setting.setting, &prev ) ) < 0 ) {
 		/* Setting not yet defined: start searching from 00:00.0 */
 		prev = 0;
 	} else {
diff --git a/src/hci/tui/settings_ui.c b/src/hci/tui/settings_ui.c
index 77ef8108..8fe13ad8 100644
--- a/src/hci/tui/settings_ui.c
+++ b/src/hci/tui/settings_ui.c
@@ -72,11 +72,16 @@ struct setting_row_widget {
 	 * Valid only for rows that lead to new settings blocks.
 	 */
 	struct settings *settings;
+	/** Configuration setting origin
+	 *
+	 * Valid only for rows that represent individual settings.
+	 */
+	struct settings *origin;
 	/** Configuration setting
 	 *
 	 * Valid only for rows that represent individual settings.
 	 */
-	struct setting *setting;
+	struct setting setting;
 	/** Screen row */
 	unsigned int row;
 	/** Screen column */
@@ -85,8 +90,6 @@ struct setting_row_widget {
 	struct edit_box editbox;
 	/** Editing in progress flag */
 	int editing;
-	/** Setting originates from this block flag */
-	int originates_here;
 	/** Buffer for setting's value */
 	char value[256]; /* enough size for a DHCP string */
 };
@@ -115,7 +118,6 @@ struct setting_widget {
 static unsigned int select_setting_row ( struct setting_widget *widget,
 					 unsigned int index ) {
 	struct settings *settings;
-	struct settings *origin;
 	struct setting *setting;
 	unsigned int count = 0;
 
@@ -147,18 +149,13 @@ static unsigned int select_setting_row ( struct setting_widget *widget,
 		if ( ! setting_applies ( widget->settings, setting ) )
 			continue;
 		if ( count++ == index ) {
-			widget->row.setting = setting;
 
-			/* Read current setting value */
-			fetchf_setting ( widget->settings, widget->row.setting,
+			/* Read current setting value and origin */
+			fetchf_setting ( widget->settings, setting,
+					 &widget->row.origin,
+					 &widget->row.setting,
 					 widget->row.value,
 					 sizeof ( widget->row.value ) );
-
-			/* Check setting's origin */
-			origin = fetch_setting_origin ( widget->settings,
-							widget->row.setting );
-			widget->row.originates_here =
-				( origin == widget->settings );
 		}
 	}
 
@@ -209,7 +206,7 @@ static void draw_setting_row ( struct setting_widget *widget ) {
 
 		/* Construct dot-padded name */
 		memset ( text.name, '.', sizeof ( text.name ) );
-		string_copy ( text.name, widget->row.setting->name,
+		string_copy ( text.name, widget->row.setting.name,
 			      sizeof ( text.name ) );
 
 		/* Construct space-padded value */
@@ -222,8 +219,10 @@ static void draw_setting_row ( struct setting_widget *widget ) {
 	}
 
 	/* Print row */
-	if ( widget->row.originates_here || widget->row.settings )
+	if ( ( widget->row.origin == widget->settings ) ||
+	     ( widget->row.settings != NULL ) ) {
 		attron ( A_BOLD );
+	}
 	mvprintw ( widget->row.row, widget->row.col, "%s", text.start );
 	attroff ( A_BOLD );
 	move ( widget->row.row, widget->row.col + curs_offset );
@@ -237,7 +236,7 @@ static void draw_setting_row ( struct setting_widget *widget ) {
  * @ret key		Key returned to application, or zero
  */
 static int edit_setting ( struct setting_widget *widget, int key ) {
-	assert ( widget->row.setting != NULL );
+	assert ( widget->row.setting.name != NULL );
 	widget->row.editing = 1;
 	return edit_editbox ( &widget->row.editbox, key );
 }
@@ -248,8 +247,8 @@ static int edit_setting ( struct setting_widget *widget, int key ) {
  * @v widget		Setting widget
  */
 static int save_setting ( struct setting_widget *widget ) {
-	assert ( widget->row.setting != NULL );
-	return storef_setting ( widget->settings, widget->row.setting,
+	assert ( widget->row.setting.name != NULL );
+	return storef_setting ( widget->settings, &widget->row.setting,
 				widget->row.value );
 }
 
@@ -344,28 +343,26 @@ static void draw_title_row ( struct setting_widget *widget ) {
  * @v widget		Setting widget
  */
 static void draw_info_row ( struct setting_widget *widget ) {
-	struct settings *origin;
 	char buf[32];
 
 	/* Draw nothing unless this row represents a setting */
 	clearmsg ( INFO_ROW );
 	clearmsg ( INFO_ROW + 1 );
-	if ( ! widget->row.setting )
+	if ( ! widget->row.setting.name )
 		return;
 
 	/* Determine a suitable setting name */
-	origin = fetch_setting_origin ( widget->settings, widget->row.setting );
-	if ( ! origin )
-		origin = widget->settings;
-	setting_name ( origin, widget->row.setting, buf, sizeof ( buf ) );
+	setting_name ( ( widget->row.origin ?
+			 widget->row.origin : widget->settings ),
+		       &widget->row.setting, buf, sizeof ( buf ) );
 
 	/* Draw row */
 	attron ( A_BOLD );
-	msg ( INFO_ROW, "%s - %s", buf, widget->row.setting->description );
+	msg ( INFO_ROW, "%s - %s", buf, widget->row.setting.description );
 	attroff ( A_BOLD );
 	color_set ( CPAIR_URL, NULL );
 	msg ( ( INFO_ROW + 1 ), "http://ipxe.org/cfg/%s",
-	      widget->row.setting->name );
+	      widget->row.setting.name );
 	color_set ( CPAIR_NORMAL, NULL );
 }
 
@@ -384,7 +381,7 @@ static void draw_instruction_row ( struct setting_widget *widget ) {
 	} else {
 		msg ( INSTRUCTION_ROW,
 		      "%sCtrl-X - exit configuration utility",
-		      ( widget->row.originates_here ?
+		      ( ( widget->row.origin == widget->settings ) ?
 			"Ctrl-D - delete setting" INSTRUCTION_PAD : "" ) );
 	}
 }
@@ -479,7 +476,7 @@ static int main_loop ( struct settings *settings ) {
 		if ( widget.row.editing ) {
 
 			/* Sanity check */
-			assert ( widget.row.setting != NULL );
+			assert ( widget.row.setting.name != NULL );
 
 			/* Redraw edit box */
 			color_set ( CPAIR_EDIT, NULL );
@@ -530,10 +527,10 @@ static int main_loop ( struct settings *settings ) {
 				move = +widget.num_rows;
 				break;
 			case CTRL_D:
-				if ( ! widget.row.setting )
+				if ( ! widget.row.setting.name )
 					break;
 				if ( ( rc = delete_setting ( widget.settings,
-						widget.row.setting ) ) != 0 ) {
+						&widget.row.setting ) ) != 0 ) {
 					alert ( " %s ", strerror ( rc ) );
 				}
 				select_setting_row ( &widget, widget.current );
@@ -550,7 +547,7 @@ static int main_loop ( struct settings *settings ) {
 				}
 				/* Fall through */
 			default:
-				if ( widget.row.setting ) {
+				if ( widget.row.setting.name ) {
 					edit_setting ( &widget, key );
 					redraw = 1;
 				}