[v2] Sort the help info shown in monitor at runtime

Introduced two queues to save sorted command list in it. As a result, command
help and help info would show a more friendly sorted command list.
For eg:
(qemu)help
acl_add
acl_policy
acl_remove
acl_reset
acl_show
balloon
block_passwd
...
the command list is sorted.

v2: write sorted command list back to original array.

Signed-off-by: Wayne Xia <xiawenc@linux.vnet.ibm.com>
---
 monitor.c |  113 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 109 insertions(+), 4 deletions(-)

Wayne Xia <xiawenc@linux.vnet.ibm.com> writes:

> Introduced two queues to save sorted command list in it. As a result, command
> help and help info would show a more friendly sorted command list.
> For eg:
> (qemu)help
> acl_add
> acl_policy
> acl_remove
> acl_reset
> acl_show
> balloon
> block_passwd
> ...
> the command list is sorted.
>
> v2: write sorted command list back to original array.
>
> Signed-off-by: Wayne Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  monitor.c |  113 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 files changed, 109 insertions(+), 4 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index 31b212a..122f950 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -66,6 +66,7 @@
>  #include "memory.h"
>  #include "qmp-commands.h"
>  #include "hmp.h"
> +#include "qemu-queue.h"
>  
>  //#define DEBUG
>  //#define DEBUG_COMPLETION
> @@ -128,6 +129,18 @@ typedef struct mon_cmd_t {
>      int flags;
>  } mon_cmd_t;
>  
> +/* queue structure used for runtime sorting */
> +struct HelpCmdElem {
> +    QTAILQ_ENTRY(HelpCmdElem) entry;
> +    mon_cmd_t cmd_copy;
> +};
> +
> +typedef struct HelpCmdElem HelpCmdElem;
> +
> +QTAILQ_HEAD(HelpCmdQueue, HelpCmdElem);
> +
> +typedef struct HelpCmdQueue HelpCmdQueue;
> +
>  /* file descriptors passed via SCM_RIGHTS */
>  typedef struct mon_fd_t mon_fd_t;
>  struct mon_fd_t {
> @@ -195,8 +208,8 @@ static inline int mon_print_count_get(const Monitor *mon) { return 0; }
>  
>  static QLIST_HEAD(mon_list, Monitor) mon_list;
>  
> -static const mon_cmd_t mon_cmds[];
> -static const mon_cmd_t info_cmds[];
> +static mon_cmd_t mon_cmds[];
> +static mon_cmd_t info_cmds[];
>  
>  static const mon_cmd_t qmp_cmds[];
>  static const mon_cmd_t qmp_query_cmds[];
> @@ -2726,13 +2739,14 @@ int monitor_get_fd(Monitor *mon, const char *fdname)
>      return -1;
>  }
>  
> -static const mon_cmd_t mon_cmds[] = {
> +/* mon_cmds and info_cmds would be sorted at runtime */
> +static mon_cmd_t mon_cmds[] = {
>  #include "hmp-commands.h"
>      { NULL, NULL, },
>  };
>  
>  /* Please update hmp-commands.hx when adding or changing commands */
> -static const mon_cmd_t info_cmds[] = {
> +static mon_cmd_t info_cmds[] = {
>      {
>          .name       = "version",
>          .args_type  = "",
> @@ -5068,6 +5082,94 @@ static void monitor_event(void *opaque, int event)
>      }
>  }
>  
> +static int cmdlist_sortto_queue(const mon_cmd_t *cmdlist,
> +                                HelpCmdQueue **pqueue)
> +{
> +    const mon_cmd_t *cmd = NULL;
> +    HelpCmdElem *elem = NULL;
> +    HelpCmdElem *newelem = NULL;
> +    HelpCmdElem *next = NULL;
> +    HelpCmdQueue *cmdqueue = NULL;
> +    int cmpret1, cmpret2;
> +
> +    cmdqueue = g_malloc(sizeof(HelpCmdQueue));

Are you sure this can fail?

> +    if (cmdqueue == NULL) {
> +        return -1;
> +    }
> +
> +    for (cmd = cmdlist; cmd->name != NULL; cmd++) {
> +        newelem = NULL;
> +        newelem = g_malloc(sizeof(HelpCmdElem));
> +        if (newelem == NULL) {
> +            return -1;
> +        }
> +        newelem->cmd_copy = *cmd;
> +
> +        if (QTAILQ_EMPTY(cmdqueue)) {
> +            QTAILQ_INSERT_HEAD(cmdqueue, newelem, entry);
> +            continue;
> +        }
> +        QTAILQ_FOREACH_SAFE(elem, cmdqueue, entry, next) {
> +            /* search for proper postion */
> +            cmpret1 = strcmp(cmd->name, elem->cmd_copy.name);
> +            if (next == NULL) {
> +                if (cmpret1 >= 0) {
> +                    QTAILQ_INSERT_TAIL(cmdqueue, newelem, entry);
> +                } else {
> +                    QTAILQ_INSERT_HEAD(cmdqueue, newelem, entry);
> +                }
> +                break;
> +            } else {
> +                cmpret2 = strcmp(cmd->name, next->cmd_copy.name);
> +            }
> +            if ((cmpret1 >= 0) && (cmpret2 <= 0)) {
> +                QTAILQ_INSERT_AFTER(cmdqueue, elem, newelem, entry);
> +                break;
> +            }
> +        }
> +    }
> +    *pqueue = cmdqueue;
> +    return 1;
> +}
> +
> +static void queue_to_cmdlist(HelpCmdQueue **pqueue,
> +                                mon_cmd_t *cmdlist)
> +{
> +    HelpCmdElem *elem = NULL;
> +    HelpCmdElem *next = NULL;
> +    HelpCmdQueue *cmdqueue = *pqueue;
> +    mon_cmd_t *pcmdlist = cmdlist;
> +
> +    QTAILQ_FOREACH_SAFE(elem, cmdqueue, entry, next) {
> +        *(pcmdlist++) = elem->cmd_copy;
> +        QTAILQ_REMOVE(cmdqueue, elem, entry);
> +        g_free(elem);
> +    }
> +    g_free(cmdqueue);
> +    *pqueue = NULL;
> +}
> +
> +static void sortcmdlist(void)
> +{
> +    HelpCmdQueue *mon_cmds_queue = NULL;
> +    HelpCmdQueue *info_cmds_queue = NULL;
> +    int ret;
> +
> +    ret = cmdlist_sortto_queue(mon_cmds, &mon_cmds_queue);
> +    if (ret < 0) {
> +        printf("error in initilize mon cmd queue, return is %d.\n", ret);

"initialize"

Error messages go to stderr.  Best use error_report().

> +        return;
> +    }
> +    queue_to_cmdlist(&mon_cmds_queue, mon_cmds);
> +
> +    ret = cmdlist_sortto_queue(info_cmds, &info_cmds_queue);
> +    if (ret < 0) {
> +        printf("error in initilize info cmd queue, return is %d.\n", ret);
> +        return;
> +    }
> +    queue_to_cmdlist(&info_cmds_queue, info_cmds);
> +}
> +
>  
>  /*
>   * Local variables:
> @@ -5110,6 +5212,9 @@ void monitor_init(CharDriverState *chr, int flags)
>      QLIST_INSERT_HEAD(&mon_list, mon, entry);
>      if (!default_mon || (flags & MONITOR_IS_DEFAULT))
>          default_mon = mon;
> +
> +    sortcmdlist();
> +
>  }
>  
>  static void bdrv_password_cb(Monitor *mon, const char *password, void *opaque)

Stupid question: what's wrong with qsort()ing in place?

于 2011-10-11 17:09, Markus Armbruster 写道:
> Wayne Xia<xiawenc@linux.vnet.ibm.com>  writes:
>
>> Introduced two queues to save sorted command list in it. As a result, command
>> help and help info would show a more friendly sorted command list.
>> For eg:
>> (qemu)help
>> acl_add
>> acl_policy
>> acl_remove
>> acl_reset
>> acl_show
>> balloon
>> block_passwd
>> ...
>> the command list is sorted.
>>
>> v2: write sorted command list back to original array.
>>
>> Signed-off-by: Wayne Xia<xiawenc@linux.vnet.ibm.com>
>> ---
>>   monitor.c |  113 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>>   1 files changed, 109 insertions(+), 4 deletions(-)
>>
>> diff --git a/monitor.c b/monitor.c
>> index 31b212a..122f950 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -66,6 +66,7 @@
>>   #include "memory.h"
>>   #include "qmp-commands.h"
>>   #include "hmp.h"
>> +#include "qemu-queue.h"
>>
>>   //#define DEBUG
>>   //#define DEBUG_COMPLETION
>> @@ -128,6 +129,18 @@ typedef struct mon_cmd_t {
>>       int flags;
>>   } mon_cmd_t;
>>
>> +/* queue structure used for runtime sorting */
>> +struct HelpCmdElem {
>> +    QTAILQ_ENTRY(HelpCmdElem) entry;
>> +    mon_cmd_t cmd_copy;
>> +};
>> +
>> +typedef struct HelpCmdElem HelpCmdElem;
>> +
>> +QTAILQ_HEAD(HelpCmdQueue, HelpCmdElem);
>> +
>> +typedef struct HelpCmdQueue HelpCmdQueue;
>> +
>>   /* file descriptors passed via SCM_RIGHTS */
>>   typedef struct mon_fd_t mon_fd_t;
>>   struct mon_fd_t {
>> @@ -195,8 +208,8 @@ static inline int mon_print_count_get(const Monitor *mon) { return 0; }
>>
>>   static QLIST_HEAD(mon_list, Monitor) mon_list;
>>
>> -static const mon_cmd_t mon_cmds[];
>> -static const mon_cmd_t info_cmds[];
>> +static mon_cmd_t mon_cmds[];
>> +static mon_cmd_t info_cmds[];
>>
>>   static const mon_cmd_t qmp_cmds[];
>>   static const mon_cmd_t qmp_query_cmds[];
>> @@ -2726,13 +2739,14 @@ int monitor_get_fd(Monitor *mon, const char *fdname)
>>       return -1;
>>   }
>>
>> -static const mon_cmd_t mon_cmds[] = {
>> +/* mon_cmds and info_cmds would be sorted at runtime */
>> +static mon_cmd_t mon_cmds[] = {
>>   #include "hmp-commands.h"
>>       { NULL, NULL, },
>>   };
>>
>>   /* Please update hmp-commands.hx when adding or changing commands */
>> -static const mon_cmd_t info_cmds[] = {
>> +static mon_cmd_t info_cmds[] = {
>>       {
>>           .name       = "version",
>>           .args_type  = "",
>> @@ -5068,6 +5082,94 @@ static void monitor_event(void *opaque, int event)
>>       }
>>   }
>>
>> +static int cmdlist_sortto_queue(const mon_cmd_t *cmdlist,
>> +                                HelpCmdQueue **pqueue)
>> +{
>> +    const mon_cmd_t *cmd = NULL;
>> +    HelpCmdElem *elem = NULL;
>> +    HelpCmdElem *newelem = NULL;
>> +    HelpCmdElem *next = NULL;
>> +    HelpCmdQueue *cmdqueue = NULL;
>> +    int cmpret1, cmpret2;
>> +
>> +    cmdqueue = g_malloc(sizeof(HelpCmdQueue));
>
> Are you sure this can fail?
>
>> +    if (cmdqueue == NULL) {
>> +        return -1;
>> +    }
>> +
>> +    for (cmd = cmdlist; cmd->name != NULL; cmd++) {
>> +        newelem = NULL;
>> +        newelem = g_malloc(sizeof(HelpCmdElem));
>> +        if (newelem == NULL) {
>> +            return -1;
>> +        }
>> +        newelem->cmd_copy = *cmd;
>> +
>> +        if (QTAILQ_EMPTY(cmdqueue)) {
>> +            QTAILQ_INSERT_HEAD(cmdqueue, newelem, entry);
>> +            continue;
>> +        }
>> +        QTAILQ_FOREACH_SAFE(elem, cmdqueue, entry, next) {
>> +            /* search for proper postion */
>> +            cmpret1 = strcmp(cmd->name, elem->cmd_copy.name);
>> +            if (next == NULL) {
>> +                if (cmpret1>= 0) {
>> +                    QTAILQ_INSERT_TAIL(cmdqueue, newelem, entry);
>> +                } else {
>> +                    QTAILQ_INSERT_HEAD(cmdqueue, newelem, entry);
>> +                }
>> +                break;
>> +            } else {
>> +                cmpret2 = strcmp(cmd->name, next->cmd_copy.name);
>> +            }
>> +            if ((cmpret1>= 0)&&  (cmpret2<= 0)) {
>> +                QTAILQ_INSERT_AFTER(cmdqueue, elem, newelem, entry);
>> +                break;
>> +            }
>> +        }
>> +    }
>> +    *pqueue = cmdqueue;
>> +    return 1;
>> +}
>> +
>> +static void queue_to_cmdlist(HelpCmdQueue **pqueue,
>> +                                mon_cmd_t *cmdlist)
>> +{
>> +    HelpCmdElem *elem = NULL;
>> +    HelpCmdElem *next = NULL;
>> +    HelpCmdQueue *cmdqueue = *pqueue;
>> +    mon_cmd_t *pcmdlist = cmdlist;
>> +
>> +    QTAILQ_FOREACH_SAFE(elem, cmdqueue, entry, next) {
>> +        *(pcmdlist++) = elem->cmd_copy;
>> +        QTAILQ_REMOVE(cmdqueue, elem, entry);
>> +        g_free(elem);
>> +    }
>> +    g_free(cmdqueue);
>> +    *pqueue = NULL;
>> +}
>> +
>> +static void sortcmdlist(void)
>> +{
>> +    HelpCmdQueue *mon_cmds_queue = NULL;
>> +    HelpCmdQueue *info_cmds_queue = NULL;
>> +    int ret;
>> +
>> +    ret = cmdlist_sortto_queue(mon_cmds,&mon_cmds_queue);
>> +    if (ret<  0) {
>> +        printf("error in initilize mon cmd queue, return is %d.\n", ret);
>
> "initialize"
>
> Error messages go to stderr.  Best use error_report().
>
right.

>> +        return;
>> +    }
>> +    queue_to_cmdlist(&mon_cmds_queue, mon_cmds);
>> +
>> +    ret = cmdlist_sortto_queue(info_cmds,&info_cmds_queue);
>> +    if (ret<  0) {
>> +        printf("error in initilize info cmd queue, return is %d.\n", ret);
>> +        return;
>> +    }
>> +    queue_to_cmdlist(&info_cmds_queue, info_cmds);
>> +}
>> +
>>
>>   /*
>>    * Local variables:
>> @@ -5110,6 +5212,9 @@ void monitor_init(CharDriverState *chr, int flags)
>>       QLIST_INSERT_HEAD(&mon_list, mon, entry);
>>       if (!default_mon || (flags&  MONITOR_IS_DEFAULT))
>>           default_mon = mon;
>> +
>> +    sortcmdlist();
>> +
>>   }
>>
>>   static void bdrv_password_cb(Monitor *mon, const char *password, void *opaque)
>
> Stupid question: what's wrong with qsort()ing in place?
>
Sorry having forgot the qsort function, would use that in next version