[settings] Eliminate calls to {fetch,store}f_named_setting() in NVO c…

…ommands A deliberate side effect of this commit is that the "read" command will now preserve the type of the setting, if the setting name contains no type information. For example: iPXE> set foo:ipv4 192.168.0.1 iPXE> read foo 192.168.0.100 iPXE> show foo foo:ipv4 = 192.168.0.100 rather than the arguably unexpected behaviour of resetting the type to "string". Signed-off-by: Michael Brown <mcb30@ipxe.org>
ipxe · Jul 22, 2013 · 652abb6 · 652abb6
1 parent 7010b10
commit 652abb6
Showing 1 changed file with 56 additions and 40 deletions.
diff --git a/src/hci/commands/nvo_cmd.c b/src/hci/commands/nvo_cmd.c
@@ -55,31 +55,44 @@ static struct command_descriptor show_cmd =
  */
 static int show_exec ( int argc, char **argv ) {
 	struct show_options opts;
-	const char *name;
+	struct named_setting setting;
+	struct settings *origin;
 	char name_buf[32];
-	char value_buf[256];
+	char *value;
 	int rc;
 
 	/* Parse options */
 	if ( ( rc = parse_options ( argc, argv, &show_cmd, &opts ) ) != 0 )
-		return rc;
+		goto err_parse_options;
 
 	/* Parse setting name */
-	name = argv[optind];
+	if ( ( rc = parse_existing_setting ( argv[optind], &setting ) ) != 0 )
+		goto err_parse_setting;
 
-	/* Fetch setting */
-	if ( ( rc = fetchf_named_setting ( name, name_buf, sizeof ( name_buf ),
-					   value_buf,
-					   sizeof ( value_buf ) ) ) < 0 ) {
+	/* Fetch formatted setting value */
+	if ( ( rc = fetchf_setting_copy ( setting.settings, &setting.setting,
+					  &value ) ) < 0 ) {
 		printf ( "Could not find \"%s\": %s\n",
-			 name, strerror ( rc ) );
-		return rc;
+			 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 */
-	printf ( "%s = %s\n", name_buf, value_buf );
+	printf ( "%s = %s\n", name_buf, value );
 
-	return 0;
+	/* Success */
+	rc = 0;
+
+	free ( value );
+ err_fetchf:
+ err_parse_setting:
+ err_parse_options:
+	return rc;
 }
 
 /** "set", "clear", and "read" options */
@@ -109,10 +122,10 @@ static struct command_descriptor clear_read_cmd =
  */
 static int set_core_exec ( int argc, char **argv,
 			   struct command_descriptor *cmd,
-			   int ( * get_value ) ( const char *name,
+			   int ( * get_value ) ( struct named_setting *setting,
 						 char **args, char **value ) ) {
 	struct set_core_options opts;
-	const char *name;
+	struct named_setting setting;
 	char *value;
 	int rc;
 
@@ -121,39 +134,44 @@ static int set_core_exec ( int argc, char **argv,
 		goto err_parse_options;
 
 	/* Parse setting name */
-	name = argv[optind];
+	if ( ( rc = parse_autovivified_setting ( argv[optind],
+						 &setting ) ) != 0 )
+		goto err_parse_setting;
 
 	/* Parse setting value */
-	if ( ( rc = get_value ( name, &argv[ optind + 1 ], &value ) ) != 0 )
+	if ( ( rc = get_value ( &setting, &argv[ optind + 1 ], &value ) ) != 0 )
 		goto err_get_value;
 
-	/* Determine total length of command line */
-	if ( ( rc = storef_named_setting ( name, &setting_type_string,
-					   value ) ) != 0 ) {
-		printf ( "Could not %s \"%s\": %s\n",
-			 argv[0], name, strerror ( rc ) );
+	/* Apply default type if necessary */
+	if ( ! setting.setting.type )
+		setting.setting.type = &setting_type_string;
+
+	/* Store setting */
+	if ( ( rc = storef_setting ( setting.settings, &setting.setting,
+				     value ) ) != 0 ) {
+		printf ( "Could not store \"%s\": %s\n",
+			 setting.setting.name, strerror ( rc ) );
 		goto err_store;
 	}
 
-	free ( value );
-	return 0;
-
  err_store:
 	free ( value );
  err_get_value:
+ err_parse_setting:
  err_parse_options:
 	return rc;
 }
 
 /**
  * Get setting value for "set" command
  *
- * @v name		Setting name
+ * @v setting		Named setting
  * @v args		Remaining arguments
  * @ret value		Setting value
  * @ret rc		Return status code
  */
-static int set_value ( const char *name __unused, char **args, char **value ) {
+static int set_value ( struct named_setting *setting __unused,
+		       char **args, char **value ) {
 
 	*value = concat_args ( args );
 	if ( ! *value )
@@ -176,13 +194,13 @@ static int set_exec ( int argc, char **argv ) {
 /**
  * Get setting value for "clear" command
  *
- * @v name		Setting name
+ * @v setting		Named setting
  * @v args		Remaining arguments
  * @ret value		Setting value
  * @ret rc		Return status code
  */
-static int clear_value ( const char *name __unused, char **args __unused,
-			 char **value ) {
+static int clear_value ( struct named_setting *setting __unused,
+			 char **args __unused, char **value ) {
 
 	*value = NULL;
 	return 0;
@@ -202,29 +220,27 @@ static int clear_exec ( int argc, char **argv ) {
 /**
  * Get setting value for "read" command
  *
- * @v name		Setting name
+ * @v setting		Named setting
  * @v args		Remaining arguments
  * @ret value		Setting value
  * @ret rc		Return status code
  */
-static int read_value ( const char *name, char **args __unused, char **value ) {
+static int read_value ( struct named_setting *setting, char **args __unused,
+			char **value ) {
 	char *existing;
 	int rc;
 
-	/* Read existing value */
-	if ( ( rc = fetchf_named_setting_copy ( name, &existing ) ) < 0 )
-		goto err_existing;
+	/* Read existing value, treating errors as equivalent to an
+	 * empty initial setting.
+	 */
+	fetchf_setting_copy ( setting->settings, &setting->setting, &existing );
 
 	/* Read new value */
 	if ( ( rc = readline_history ( NULL, existing, NULL, value ) ) != 0 )
-		goto err_new;
-
-	/* Success */
-	rc = 0;
+		goto err_readline;
 
- err_new:
+ err_readline:
 	free ( existing );
- err_existing:
 	return rc;
 }