Re: [PATCH 3/3] Build command tries recusively

[Jose E. Marchesi]

    We can use the presence or absence of the subcommand list to determine if
    the subtrie needs to be initialised rather than open-coding it. This makes
    the command init process a bit simpler and self-synchronsing.
    
    Signed-off-by: Oliver O'Halloran <address@hidden>
    ---
     src/pk-cmd.c  | 43 +++++++++++++++++--------------------------
     src/pk-cmd.h  |  2 +-
     src/pk-help.c |  4 +---
     src/pk-info.c |  4 +---
     src/pk-set.c  |  4 +---
     src/pk-vm.c   |  8 ++------
     6 files changed, 23 insertions(+), 42 deletions(-)
    
    diff --git a/src/pk-cmd.c b/src/pk-cmd.c
    index 7500471..4a53bb8 100644
    --- a/src/pk-cmd.c
    +++ b/src/pk-cmd.c
    @@ -199,8 +199,13 @@ pk_trie_from_cmds (struct pk_cmd *cmds[])
           /* Note this assumes no commands with empty names.  */
           t->cmd = cmd;
           t = root;
    +
    +      /* if we have subcommands, expand their trie too */
    +      if (cmd->subcmds)
    +        cmd->subtrie = pk_trie_from_cmds(cmd->subcmds);

Please leave a space between the function name and the argument list.

        }
     
    +  /* associate the command with all the intermediary trie nodes */
       pk_trie_expand_cmds (root, root);
       return root;
     }
    @@ -312,7 +317,7 @@ pk_cmd_exec_1 (char *str, struct pk_trie *cmds_trie, 
char *prefix)
             goto usage;
     
           if (*p != '\0')
    -        return pk_cmd_exec_1 (p, *cmd->subtrie, cmd_name);
    +        return pk_cmd_exec_1 (p, cmd->subtrie, cmd_name);
         }
       
       /* Parse arguments.  */
    @@ -585,21 +590,6 @@ pk_cmd_exec_1 (char *str, struct pk_trie *cmds_trie, 
char *prefix)
       return 0;
     }
     
    -extern struct pk_cmd *info_cmds[]; /* pk-info.c  */
    -extern struct pk_trie *info_trie; /* pk-info.c  */
    -
    -extern struct pk_cmd *help_cmds[]; /* pk-help.c */
    -extern struct pk_trie *help_trie; /* pk-help.c */
    -
    -extern struct pk_cmd *vm_cmds[]; /* pk-vm.c  */
    -extern struct pk_trie *vm_trie;  /* pk-vm.c  */
    -
    -extern struct pk_cmd *vm_disas_cmds[];  /* pk-vm.c */
    -extern struct pk_trie *vm_disas_trie; /* pk-vm.c */
    -
    -extern struct pk_cmd *set_cmds[]; /* pk-set.c */
    -extern struct pk_trie *set_trie; /* pk-set.c */
    -
     static struct pk_trie *cmds_trie;
     
     int
    @@ -747,11 +737,6 @@ void
     pk_cmd_init (void)
     {
       cmds_trie = pk_trie_from_cmds (cmds);
    -  info_trie = pk_trie_from_cmds (info_cmds);
    -  help_trie = pk_trie_from_cmds (help_cmds);
    -  vm_trie = pk_trie_from_cmds (vm_cmds);
    -  vm_disas_trie = pk_trie_from_cmds (vm_disas_cmds);
    -  set_trie = pk_trie_from_cmds (set_cmds);
     
       /* Compile commands written in Poke.  */
       {
    @@ -779,10 +764,16 @@ pk_cmd_init (void)
     void
     pk_cmd_shutdown (void)
     {
    +  struct pk_cmd *cmd;
    +  int i;
    +
    +  for (i = 0, cmd = cmds[0];
    +       cmd->name != NULL;
    +       cmd = cmds[++i])
    +    {
    +      if (cmd->subtrie)
    +        pk_trie_free (cmd->subtrie);
    +    }
    +
       pk_trie_free (cmds_trie);
    -  pk_trie_free (info_trie);
    -  pk_trie_free (help_trie);
    -  pk_trie_free (vm_trie);
    -  pk_trie_free (vm_disas_trie);
    -  pk_trie_free (set_trie);
     }

Hm, but this won't work recursively right?  i.e. what about
vm_disas_trie.

    diff --git a/src/pk-cmd.h b/src/pk-cmd.h
    index f498ce2..956e662 100644
    --- a/src/pk-cmd.h
    +++ b/src/pk-cmd.h
    @@ -82,7 +82,7 @@ struct pk_cmd
       const char *usage;
     
       /* tail element that we don't need to initialise */
    -  struct pk_trie **subtrie;
    +  struct pk_trie *subtrie;
     };
     
     /* Parse STR and execute a command.  */
    diff --git a/src/pk-help.c b/src/pk-help.c
    index 622c3ac..199d984 100644
    --- a/src/pk-help.c
    +++ b/src/pk-help.c
    @@ -44,7 +44,5 @@ struct pk_cmd *help_cmds[] =
         &null_cmd
       };
     
    -struct pk_trie *help_trie;
    -
     struct pk_cmd help_cmd =
    -  {"help", "", "", 0, help_cmds, list_cmds_fn, "help", &help_trie};
    +  {"help", "", "", 0, help_cmds, list_cmds_fn, "help"};
    diff --git a/src/pk-info.c b/src/pk-info.c
    index 28f88b7..b980ed7 100644
    --- a/src/pk-info.c
    +++ b/src/pk-info.c
    @@ -32,7 +32,5 @@ struct pk_cmd *info_cmds[] =
         &null_cmd
       };
     
    -struct pk_trie *info_trie;
    -
     struct pk_cmd info_cmd =
    -  {"info", "", "", 0, info_cmds, NULL, "info (files|variable|type)", 
&info_trie};
    +  {"info", "", "", 0, info_cmds, NULL, "info (files|variable|type)"};
    diff --git a/src/pk-set.c b/src/pk-set.c
    index 354d1d3..ec6f5e6 100644
    --- a/src/pk-set.c
    +++ b/src/pk-set.c
    @@ -194,7 +194,5 @@ struct pk_cmd *set_cmds[] =
        &null_cmd
       };
     
    -struct pk_trie *set_trie;
    -
     struct pk_cmd set_cmd =
    -  {"set", "", "", 0, set_cmds, NULL, "set PROPERTY", &set_trie};
    +  {"set", "", "", 0, set_cmds, NULL, "set PROPERTY"};
    diff --git a/src/pk-vm.c b/src/pk-vm.c
    index b810572..0c9dadc 100644
    --- a/src/pk-vm.c
    +++ b/src/pk-vm.c
    @@ -198,11 +198,9 @@ struct pk_cmd *vm_disas_cmds[] =
        &null_cmd
       };
     
    -struct pk_trie *vm_disas_trie;
    -
     struct pk_cmd vm_disas_cmd =
       {"disassemble", "e", PK_VM_DIS_UFLAGS, 0, vm_disas_cmds, NULL,
    -   "vm disassemble (expression|function)", &vm_disas_trie};
    +   "vm disassemble (expression|function)"};
     
     struct pk_cmd *vm_cmds[] =
       {
    @@ -210,7 +208,5 @@ struct pk_cmd *vm_cmds[] =
         &null_cmd
       };
     
    -struct pk_trie *vm_trie;
    -
     struct pk_cmd vm_cmd =
    -  {"vm", "", "", 0, vm_cmds, NULL, "vm (disassemble)", &vm_trie};
    +  {"vm", "", "", 0, vm_cmds, NULL, "vm (disassemble)"};