Skip to content

Commit

Permalink
host-utils: Proactively fix pow2floor(), switch to unsigned
Browse files Browse the repository at this point in the history
The function's stated contract is simple enough: "round down to the
nearest power of 2".  Suggests the domain is the representable numbers
>= 1, because that's the smallest power of two.

The implementation doesn't check for domain errors, but returns
garbage instead:

* For negative arguments, pow2floor() returns -2^63, which is not even
  a power of two, let alone the nearest one.

  What sort of works is passing *unsigned* arguments >= 2^63.  The
  implicit conversion to signed is implementation defined, but
  commonly yields the (negative) two's complement.  pow2floor() then
  returns -2^63.  Callers that convert that back to unsigned get the
  correct value 2^63.

* For a zero argument, pow2floor() shifts right by 64.  Undefined
  behavior.  Common actual behavior is to shift by 0, yielding -2^63.

Fix by switching from int64_t to uint64_t and amending the contract to
map zero to zero.

Callers are fine with that:

* memory_access_size()

  This function makes no sense unless the argument is positive and the
  return value fits into int.

* raw_refresh_limits()

  Passes an int between 1 and BDRV_REQUEST_MAX_BYTES.

* iscsi_refresh_limits()

  Passes an integer between 0 and INT_MAX, converts the result to
  uint32_t.  Passing zero would be undefined behavior, but commonly
  yield zero.  The patch gives us the zero without the undefined
  behavior.

* cache_init()

  Passes a positive int64_t argument.

* xbzrle_cache_resize()

  Passes a positive int64_t argument (>= TARGET_PAGE_SIZE, actually).

* spapr_node0_size()

  Passes a positive uint64_t argument, and converts the result to
  hwaddr, i.e. uint64_t.

* spapr_populate_memory()

  Passes a positive hwaddr argument, and converts the result to
  hwaddr.

Cc: Juan Quintela <quintela@redhat.com>
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Cc: Eric Blake <eblake@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1501148776-16890-3-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
  • Loading branch information
Markus Armbruster authored and dagrh committed Sep 6, 2017
1 parent d0834eb commit 43c64a0
Showing 1 changed file with 8 additions and 5 deletions.
13 changes: 8 additions & 5 deletions include/qemu/host-utils.h
Expand Up @@ -369,13 +369,16 @@ static inline bool is_power_of_2(uint64_t value)
return !(value & (value - 1));
}

/* round down to the nearest power of 2*/
static inline int64_t pow2floor(int64_t value)
/**
* Return @value rounded down to the nearest power of two or zero.
*/
static inline uint64_t pow2floor(uint64_t value)
{
if (!is_power_of_2(value)) {
value = 0x8000000000000000ULL >> clz64(value);
if (!value) {
/* Avoid undefined shift by 64 */
return 0;
}
return value;
return 0x8000000000000000ull >> clz64(value);
}

/* round up to the nearest power of 2 (0 if overflow) */
Expand Down

0 comments on commit 43c64a0

Please sign in to comment.