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

Re: [Xen-devel] [PATCH] xl: Don't require a config file for cpupools



George Dunlap writes ("[Xen-devel] [PATCH] xl: Don't require a config file for 
cpupools"):
> Since the key information can be fairly simply put on the command-line,
> there's no need to require an actual config file.

Thanks.  Just a few tiny coding style nits.  I agree with Ian
Campbell's comments, and also:

> -    const char *filename = NULL;
> +    const char *filename = NULL, *config_src=NULL;

This has inconsistent use of whitespace.

> +    if (filename)
> +    {

This should have { on the same line as the if.

> +        if (libxl_read_file_contents(ctx, filename, (void **)&config_data,
> +                                     &config_len)) {
> +            fprintf(stderr, "Failed to read config file: %s: %s\n",
> +                    filename, strerror(errno));
> +            return -ERROR_FAIL;
> +        }
> +        config_src=filename;

We put spaces around "=".  (Here and a few lines further on.)

> +    }
> +    else

And on the same line as the else.  And having { } for the if, I think
putting { } for the else block would be more conventional.

> -    printf("Using config file \"%s\"\n", filename);
> +    printf("Using config file \"%s\"\n", config_src);

This will print
   Using config file "command line"
which is rather an odd message.  Perhaps change the string to
`<command line>' and remove the quotes ?

Ian.

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.