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

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>

diff --git a/src/hci/commands/nvo_cmd.c b/src/hci/commands/nvo_cmd.c
index f255bbf5742bfc2d0b23b21b2194ee5e22c2a844..dd54a84e111138615296352f82c9fcf637588442 100644 (file)
--- 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,26 +134,30 @@ 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;
 }
@@ -148,12 +165,13 @@ static int set_core_exec ( int argc, char **argv,
 /**
  * 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;
 }