Re: [PATCH] fix uint64 underflow

[Michal Prívozník <mprivozn@xxxxxxxxxx>]

On 9/12/23 16:50, Dmitry Frolov wrote:
> According to previous statement,
> 'free_mem' is less than 'needed_mem'.
> So, the subtraction 'free_mem - needed_mem' is negative,
> and will raise uint64 underflow.
> 
> Signed-off-by: Dmitry Frolov <frolov@xxxxxxxxx>
> ---
>  src/libxl/libxl_domain.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> index 6c167df63e..36be042971 100644
> --- a/src/libxl/libxl_domain.c
> +++ b/src/libxl/libxl_domain.c
> @@ -940,7 +940,7 @@ libxlDomainFreeMem(libxl_ctx *ctx, libxl_domain_config *d_config)
>          if (free_mem >= needed_mem)
>              return 0;
>  
> -        target_mem = free_mem - needed_mem;
> +        target_mem = needed_mem - free_mem;
>          if (libxlSetMemoryTargetWrapper(ctx, 0, target_mem,
>                                          /* relative */ 1, 0) < 0)
>              goto error;

Yeah, this fixes the underflow, but I worry about the while semantics a
bit. What the code effectively does:

  libxl_domain_need_memory(&needed_mem);

  do {
    libxl_get_free_memory(&free_mem);

    if (free_mem >= needed_mem)
      return 0;

    target_mem = needed_mem - free_mem;
    libxl_set_memory_target(target_mem);
  } while(...)

Now, if libxl_domain_need_memory() returns how much memory a domain
really needs, then why undergo trouble of getting its free memory? Why
not pass it to set_memory_target() right away?

Or am I misunderstanding something?

Michal