Re: [PATCH 1/9] xl: Improve return and exit codes of memory related functions.

[Dario Faggioli <dario.faggioli@citrix.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2308 bytes --]

On Wed, 2016-02-24 at 18:23 +0530, Harmandeep Kaur wrote:
> Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com>
> ---
>  tools/libxl/xl_cmdimpl.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 116363d..8f5a2f4 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -172,7 +172,7 @@ static uint32_t find_domain(const char *p)
>      rc = libxl_domain_qualifier_to_domid(ctx, p, &domid);
>      if (rc) {
>          fprintf(stderr, "%s is an invalid domain identifier
> (rc=%d)\n", p, rc);
> -        exit(2);
> +        exit(EXIT_FAILURE);
>
I'm not sure find_domain() is a "memory related functions", so I
wouldn't change it in this patch.

On the other hand, there is parse_mem_size_kb(), which returns -1 on
failure, or the amount of memory, on success.

That is ok, IMO, so, don't change it. However, I think you can take the
chance of this patch to add just a couple of line of comments, above
its signature, explaining that it does exactly that.

Also, there is freemem(), which also does look "memory related" to me.
And in its case, it indeed uses libxl's error codes while it should --
being an internal function-- just use the 0/1 convention. I think you
can 'fix' it in this patch.

> @@ -3275,7 +3275,7 @@ static int set_memory_max(uint32_t domid, const
> char *mem)
>      memorykb = parse_mem_size_kb(mem);
>      if (memorykb == -1) {
>          fprintf(stderr, "invalid memory size: %s\n", mem);
> -        exit(3);
> +        exit(EXIT_FAILURE);
>      }
>  
>      rc = libxl_domain_setmaxmem(ctx, domid, memorykb);
>
The statement right below this one is:

 return rc;

set_memory_max() is an internal function so, again, it should retunr
0/1 to its caller, rather than a libxl error code.

The rest of this patch looks ok to me.

Thanks and Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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