Re: [Xen-devel] [PATCH v3 4/7] xenstore: enhance control command support

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Wed, Feb 22, 2017 at 04:28:48PM +0100, Juergen Gross wrote:
> The Xenstore protocol supports the XS_CONTROL command for triggering
> various actions in the Xenstore daemon. Enhance that support by using
> a command table and adding a help function.
> 
> Support multiple control commands in the associated xenstore-control
> program used to issue XS_CONTROL commands.
> 
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> ---
>  tools/xenstore/xenstore_control.c  |  65 +++++++++++++++++-------
>  tools/xenstore/xenstored_control.c | 100 
> +++++++++++++++++++++++++++++++++----
>  2 files changed, 135 insertions(+), 30 deletions(-)
> 
> diff --git a/tools/xenstore/xenstore_control.c 
> b/tools/xenstore/xenstore_control.c
> index 0a108df..e42d478 100644
> --- a/tools/xenstore/xenstore_control.c
> +++ b/tools/xenstore/xenstore_control.c
> @@ -7,29 +7,56 @@
>  
>  int main(int argc, char **argv)
>  {
> -  struct xs_handle * xsh;
> +    struct xs_handle *xsh;
> +    char *par = NULL;
> +    char *ret;
> +    unsigned int p, len = 0;
> +    int rc = 0;
>  
> -  if (argc < 2 ||
> -      strcmp(argv[1], "check"))
> -  {
> -    fprintf(stderr,
> -            "Usage:\n"
> -            "\n"
> -            "       %s check\n"
> -            "\n", argv[0]);
> -    return 2;
> -  }
> +    if (argc < 2) {
> +        fprintf(stderr, "Usage:\n"
> +                "%s <command> [<arg>...]\n", argv[0]);
> +        return 2;
> +    }
>  
> -  xsh = xs_daemon_open();
> +    for (p = 2; p < argc; p++)
> +        len += strlen(argv[p]) + 1;
> +    if (len) {
> +        par = malloc(len);

par is never freed.

I would also suggest to use goto style error handling.

> +        if (!par) {
> +            fprintf(stderr, "Allocation error.\n");
> +            return 1;
> +        }
> +        len = 0;
> +        for (p = 2; p < argc; p++) {
> +            memcpy(par + len, argv[p], strlen(argv[p]) + 1);
> +            len += strlen(argv[p]) + 1;
> +        }
> +    }
>  
> -  if (xsh == NULL) {
> -    fprintf(stderr, "Failed to contact Xenstored.\n");
> -    return 1;
> -  }
> +    xsh = xs_open(0);
> +    if (xsh == NULL) {
> +        fprintf(stderr, "Failed to contact Xenstored.\n");
> +        return 1;
> +    }
>  
> -  xs_debug_command(xsh, argv[1], NULL, 0);
> +    ret = xs_debug_command(xsh, argv[1], par, len);

Should be xs_control_command now.

> +    if (!ret) {
> +        rc = 3;
> +        if (errno == EINVAL) {
> +            ret = xs_debug_command(xsh, "help", NULL, 0);

The rest looks good.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel