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

[Juergen Gross <jgross@xxxxxxxx>]

On 23/02/17 12:06, Wei Liu wrote:
> 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.

You are right. Will correct.

> I would also suggest to use goto style error handling.

Okay.

>> +        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.

Of course. Will change.

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

Thanks,


Juergen


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