[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 2/8] virsh: Add vshDomainCompleter



On 08.08.2013 16:38, Tomas Meszaros wrote:
> Function vshDomainCompler returns domains names.
> 
> Michal Privoznik recommended to add global variable __my_conn
> so we can get the list of domains from the virConnecTListAllDomains().
> 
> vshReconnect() is called before the first command is executed
> in order to provide autocompletion for the very first command.
> ---
>  tools/virsh.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tools/virsh.h |  2 ++
>  2 files changed, 55 insertions(+)
> 
> diff --git a/tools/virsh.c b/tools/virsh.c
> index f65dc79..af31b9a 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -88,6 +88,8 @@ static char *progname;
>  
>  static const vshCmdGrp cmdGroups[];
>  
> +virConnectPtr *__my_conn;
> +

Sigh. Unfortunately, we can't tunnel a pointer into readline's
callbacks. While I hate global variables, this one makes sense.

>  /* Bypass header poison */
>  #undef strdup
>  
> @@ -2502,6 +2504,51 @@ vshCloseLogFile(vshControl *ctl)
>  
>  #ifdef USE_READLINE
>  
> +/* -------------
> + * Completers
> + * -------------
> + */
> +
> +char **
> +vshDomainCompleter(unsigned int flags)
> +{
> +    virDomainPtr *domains;
> +    size_t i;
> +    char **names = NULL;
> +    int ndomains;
> +
> +    if (!*__my_conn)
> +        return NULL;
> +
> +    ndomains = virConnectListAllDomains(*__my_conn, &domains, flags);
> +
> +    if (ndomains < 0)
> +        return NULL;
> +
> +    names = vshMalloc(NULL, sizeof(char *) * (ndomains + 1));
> +
> +    if (!names)
> +        return NULL;

if vshMalloc fails, you leak domains[].

> +
> +    for (i = 0; i < ndomains; i++) {
> +        char *name = (char *)virDomainGetName(domains[i]);

what about making @name const char *?

> +        if (VIR_STRDUP(names[i], name) < 0) {
> +            virDomainFree(domains[i]);
> +            goto cleanup;
> +        }
> +        virDomainFree(domains[i]);
> +    }
> +    names[i] = NULL;
> +    VIR_FREE(domains);
> +    return names;
> +
> +cleanup:

This should be rather error: as the path is not common for success and
fail cases. The path is taken only on error.

> +    for (i = 0; names[i]; i++)
> +        VIR_FREE(names[i]);
> +    VIR_FREE(names);
> +    return NULL;
> +}
> +
>  /* -----------------
>   * Readline stuff
>   * -----------------
> @@ -3158,6 +3205,7 @@ main(int argc, char **argv)
>      ctl->debug = VSH_DEBUG_DEFAULT;
>      ctl->escapeChar = "^]";     /* Same default as telnet */
>  
> +    __my_conn = &ctl->conn;
>  
>      if (!setlocale(LC_ALL, "")) {
>          perror("setlocale");
> @@ -3219,6 +3267,11 @@ main(int argc, char **argv)
>              exit(EXIT_FAILURE);
>          }
>  
> +        /* Need to connect immediately after start in order to provide
> +         * autocompletion for the very first command.
> +         */
> +        vshReconnect(ctl);
> +
>          do {
>              const char *prompt = ctl->readonly ? VSH_PROMPT_RO : VSH_PROMPT_RW;
>              ctl->cmdstr =
> diff --git a/tools/virsh.h b/tools/virsh.h
> index e07b546..c4a9c13 100644
> --- a/tools/virsh.h
> +++ b/tools/virsh.h
> @@ -254,6 +254,8 @@ struct _vshCmdGrp {
>      const vshCmdDef *commands;
>  };
>  
> +char **vshDomainCompleter(unsigned int flags);
> +
>  void vshError(vshControl *ctl, const char *format, ...)
>      ATTRIBUTE_FMT_PRINTF(2, 3);
>  void vshOpenLogFile(vshControl *ctl);
> 

Michal


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]