qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH RFC 2/4] range: Eliminate direct Range member ac


From: Michael S. Tsirkin
Subject: Re: [Qemu-devel] [PATCH RFC 2/4] range: Eliminate direct Range member access
Date: Sun, 19 Jun 2016 06:25:19 +0300

On Wed, Jun 15, 2016 at 10:41:48PM +0200, Markus Armbruster wrote:
> Users of struct Range mess liberally with its members, which makes
> refactoring hard.  Create a set of methods, and convert all users to
> call them instead of accessing members.  The methods have carefully
> worded contracts, and use assertions to check them.
> 
> To help with tracking down the places that access members of struct
> Range directly, hide the implementation of struct Range outside of
> range.c by trickery.  The trickery will be dropped in the next commit.
> 
> Signed-off-by: Markus Armbruster <address@hidden>

Reviewed-by: Michael S. Tsirkin <address@hidden>

I guess you want me to merge this because of the changes in pc and pci?

> ---
>  hw/i386/acpi-build.c         |  35 +++++++-------
>  hw/pci-host/piix.c           |  26 +++++++----
>  hw/pci-host/q35.c            |  41 +++++++++++------
>  hw/pci/pci.c                 |  17 +++----
>  include/qemu/range.h         | 106 
> ++++++++++++++++++++++++++++++++++++++++++-
>  qapi/string-input-visitor.c  |  20 ++++----
>  qapi/string-output-visitor.c |  18 ++++----
>  util/log.c                   |   5 +-
>  util/range.c                 |   4 +-
>  9 files changed, 198 insertions(+), 74 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 02fc534..6c36c24 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -232,18 +232,20 @@ static void acpi_get_pci_holes(Range *hole, Range 
> *hole64)
>      pci_host = acpi_get_i386_pci_host();
>      g_assert(pci_host);
>  
> -    hole->begin = object_property_get_int(pci_host,
> -                                          PCI_HOST_PROP_PCI_HOLE_START,
> -                                          NULL);
> -    hole->end = object_property_get_int(pci_host,
> -                                        PCI_HOST_PROP_PCI_HOLE_END,
> -                                        NULL);
> -    hole64->begin = object_property_get_int(pci_host,
> -                                            PCI_HOST_PROP_PCI_HOLE64_START,
> -                                            NULL);
> -    hole64->end = object_property_get_int(pci_host,
> -                                          PCI_HOST_PROP_PCI_HOLE64_END,
> -                                          NULL);
> +    range_set_bounds1(hole,
> +                      object_property_get_int(pci_host,
> +                                              PCI_HOST_PROP_PCI_HOLE_START,
> +                                              NULL),
> +                      object_property_get_int(pci_host,
> +                                              PCI_HOST_PROP_PCI_HOLE_END,
> +                                              NULL));
> +    range_set_bounds1(hole64,
> +                      object_property_get_int(pci_host,
> +                                              PCI_HOST_PROP_PCI_HOLE64_START,
> +                                              NULL),
> +                      object_property_get_int(pci_host,
> +                                              PCI_HOST_PROP_PCI_HOLE64_END,
> +                                              NULL));
>  }
>  
>  #define ACPI_PORT_SMI_CMD           0x00b2 /* TODO: this is APM_CNT_IOPORT */
> @@ -2015,7 +2017,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>                           0, 0x000A0000, 0x000BFFFF, 0, 0x00020000));
>  
>      crs_replace_with_free_ranges(mem_ranges,
> -                                 pci_hole->begin, pci_hole->end - 1);
> +                                 range_lob(pci_hole),
> +                                 range_upb(pci_hole));
>      for (i = 0; i < mem_ranges->len; i++) {
>          entry = g_ptr_array_index(mem_ranges, i);
>          aml_append(crs,
> @@ -2025,12 +2028,12 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>                               0, entry->limit - entry->base + 1));
>      }
>  
> -    if (pci_hole64->begin) {
> +    if (!range_is_empty(pci_hole64)) {
>          aml_append(crs,
>              aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
>                               AML_CACHEABLE, AML_READ_WRITE,
> -                             0, pci_hole64->begin, pci_hole64->end - 1, 0,
> -                             pci_hole64->end - pci_hole64->begin));
> +                             0, range_lob(pci_hole64), 
> range_upb(pci_hole64), 0,
> +                             range_upb(pci_hole64) + 1 - 
> range_lob(pci_hole64)));
>      }
>  
>      if (misc->tpm_version != TPM_VERSION_UNSPEC) {
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index 8db0f09..1df327f 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -221,8 +221,12 @@ static void i440fx_pcihost_get_pci_hole_start(Object 
> *obj, Visitor *v,
>                                                Error **errp)
>  {
>      I440FXState *s = I440FX_PCI_HOST_BRIDGE(obj);
> -    uint32_t value = s->pci_hole.begin;
> +    uint64_t val64;
> +    uint32_t value;
>  
> +    val64 = range_is_empty(&s->pci_hole) ? 0 : range_lob(&s->pci_hole);
> +    value = val64;
> +    assert(value == val64);
>      visit_type_uint32(v, name, &value, errp);
>  }
>  
> @@ -231,8 +235,12 @@ static void i440fx_pcihost_get_pci_hole_end(Object *obj, 
> Visitor *v,
>                                              Error **errp)
>  {
>      I440FXState *s = I440FX_PCI_HOST_BRIDGE(obj);
> -    uint32_t value = s->pci_hole.end;
> +    uint64_t val64;
> +    uint32_t value;
>  
> +    val64 = range_is_empty(&s->pci_hole) ? 0 : range_upb(&s->pci_hole) + 1;
> +    value = val64;
> +    assert(value == val64);
>      visit_type_uint32(v, name, &value, errp);
>  }
>  
> @@ -242,10 +250,11 @@ static void i440fx_pcihost_get_pci_hole64_start(Object 
> *obj, Visitor *v,
>  {
>      PCIHostState *h = PCI_HOST_BRIDGE(obj);
>      Range w64;
> +    uint64_t value;
>  
>      pci_bus_get_w64_range(h->bus, &w64);
> -
> -    visit_type_uint64(v, name, &w64.begin, errp);
> +    value = range_is_empty(&w64) ? 0 : range_lob(&w64);
> +    visit_type_uint64(v, name, &value, errp);
>  }
>  
>  static void i440fx_pcihost_get_pci_hole64_end(Object *obj, Visitor *v,
> @@ -254,10 +263,11 @@ static void i440fx_pcihost_get_pci_hole64_end(Object 
> *obj, Visitor *v,
>  {
>      PCIHostState *h = PCI_HOST_BRIDGE(obj);
>      Range w64;
> +    uint64_t value;
>  
>      pci_bus_get_w64_range(h->bus, &w64);
> -
> -    visit_type_uint64(v, name, &w64.end, errp);
> +    value = range_is_empty(&w64) ? 0 : range_upb(&w64) + 1;
> +    visit_type_uint64(v, name, &value, errp);
>  }
>  
>  static void i440fx_pcihost_initfn(Object *obj)
> @@ -344,8 +354,8 @@ PCIBus *i440fx_init(const char *host_type, const char 
> *pci_type,
>      f->ram_memory = ram_memory;
>  
>      i440fx = I440FX_PCI_HOST_BRIDGE(dev);
> -    i440fx->pci_hole.begin = below_4g_mem_size;
> -    i440fx->pci_hole.end = IO_APIC_DEFAULT_ADDRESS;
> +    range_set_bounds(&i440fx->pci_hole, below_4g_mem_size,
> +                     IO_APIC_DEFAULT_ADDRESS - 1);
>  
>      /* setup pci memory mapping */
>      pc_pci_as_mapping_init(OBJECT(f), f->system_memory,
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index 8c2c1db..d200091 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -73,8 +73,13 @@ static void q35_host_get_pci_hole_start(Object *obj, 
> Visitor *v,
>                                          Error **errp)
>  {
>      Q35PCIHost *s = Q35_HOST_DEVICE(obj);
> -    uint32_t value = s->mch.pci_hole.begin;
> +    uint64_t val64;
> +    uint32_t value;
>  
> +    val64 = range_is_empty(&s->mch.pci_hole)
> +        ? 0 : range_lob(&s->mch.pci_hole);
> +    value = val64;
> +    assert(value == val64);
>      visit_type_uint32(v, name, &value, errp);
>  }
>  
> @@ -83,8 +88,13 @@ static void q35_host_get_pci_hole_end(Object *obj, Visitor 
> *v,
>                                        Error **errp)
>  {
>      Q35PCIHost *s = Q35_HOST_DEVICE(obj);
> -    uint32_t value = s->mch.pci_hole.end;
> +    uint64_t val64;
> +    uint32_t value;
>  
> +    val64 = range_is_empty(&s->mch.pci_hole)
> +        ? 0 : range_upb(&s->mch.pci_hole) + 1;
> +    value = val64;
> +    assert(value == val64);
>      visit_type_uint32(v, name, &value, errp);
>  }
>  
> @@ -94,10 +104,11 @@ static void q35_host_get_pci_hole64_start(Object *obj, 
> Visitor *v,
>  {
>      PCIHostState *h = PCI_HOST_BRIDGE(obj);
>      Range w64;
> +    uint64_t value;
>  
>      pci_bus_get_w64_range(h->bus, &w64);
> -
> -    visit_type_uint64(v, name, &w64.begin, errp);
> +    value = range_is_empty(&w64) ? 0 : range_lob(&w64);
> +    visit_type_uint64(v, name, &value, errp);
>  }
>  
>  static void q35_host_get_pci_hole64_end(Object *obj, Visitor *v,
> @@ -106,10 +117,11 @@ static void q35_host_get_pci_hole64_end(Object *obj, 
> Visitor *v,
>  {
>      PCIHostState *h = PCI_HOST_BRIDGE(obj);
>      Range w64;
> +    uint64_t value;
>  
>      pci_bus_get_w64_range(h->bus, &w64);
> -
> -    visit_type_uint64(v, name, &w64.end, errp);
> +    value = range_is_empty(&w64) ? 0 : range_upb(&w64) + 1;
> +    visit_type_uint64(v, name, &value, errp);
>  }
>  
>  static void q35_host_get_mmcfg_size(Object *obj, Visitor *v, const char 
> *name,
> @@ -182,9 +194,9 @@ static void q35_host_initfn(Object *obj)
>       * it's not a power of two, which means an MTRR
>       * can't cover it exactly.
>       */
> -    s->mch.pci_hole.begin = MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT +
> -        MCH_HOST_BRIDGE_PCIEXBAR_MAX;
> -    s->mch.pci_hole.end = IO_APIC_DEFAULT_ADDRESS;
> +    range_set_bounds(&s->mch.pci_hole,
> +            MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT + MCH_HOST_BRIDGE_PCIEXBAR_MAX,
> +            IO_APIC_DEFAULT_ADDRESS - 1);
>  }
>  
>  static const TypeInfo q35_host_info = {
> @@ -252,10 +264,7 @@ static void mch_update_pciexbar(MCHPCIState *mch)
>          break;
>      case MCH_HOST_BRIDGE_PCIEXBAR_LENGTH_RVD:
>      default:
> -        enable = 0;
> -        length = 0;
>          abort();
> -        break;
>      }
>      addr = pciexbar & addr_mask;
>      pcie_host_mmcfg_update(pehb, enable, addr, length);
> @@ -265,9 +274,13 @@ static void mch_update_pciexbar(MCHPCIState *mch)
>       * which means an MTRR can't cover it exactly.
>       */
>      if (enable) {
> -        mch->pci_hole.begin = addr + length;
> +        range_set_bounds(&mch->pci_hole,
> +                         addr + length,
> +                         IO_APIC_DEFAULT_ADDRESS - 1);
>      } else {
> -        mch->pci_hole.begin = MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT;
> +        range_set_bounds(&mch->pci_hole,
> +                         MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT,
> +                         IO_APIC_DEFAULT_ADDRESS - 1);
>      }
>  }
>  
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index bb605ef..904c6fd 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -2436,13 +2436,13 @@ static void pci_dev_get_w64(PCIBus *b, PCIDevice 
> *dev, void *opaque)
>  
>          if (limit >= base) {
>              Range pref_range;
> -            pref_range.begin = base;
> -            pref_range.end = limit + 1;
> +            range_set_bounds(&pref_range, base, limit);
>              range_extend(range, &pref_range);
>          }
>      }
>      for (i = 0; i < PCI_NUM_REGIONS; ++i) {
>          PCIIORegion *r = &dev->io_regions[i];
> +        pcibus_t lob, upb;
>          Range region_range;
>  
>          if (!r->size ||
> @@ -2450,16 +2450,17 @@ static void pci_dev_get_w64(PCIBus *b, PCIDevice 
> *dev, void *opaque)
>              !(r->type & PCI_BASE_ADDRESS_MEM_TYPE_64)) {
>              continue;
>          }
> -        region_range.begin = pci_bar_address(dev, i, r->type, r->size);
> -        region_range.end = region_range.begin + r->size;
>  
> -        if (region_range.begin == PCI_BAR_UNMAPPED) {
> +        lob = pci_bar_address(dev, i, r->type, r->size);
> +        upb = lob + r->size - 1;
> +        if (lob == PCI_BAR_UNMAPPED) {
>              continue;
>          }
>  
> -        region_range.begin = MAX(region_range.begin, 0x1ULL << 32);
> +        lob = MAX(lob, 0x1ULL << 32);
>  
> -        if (region_range.end - 1 >= region_range.begin) {
> +        if (upb >= lob) {
> +            range_set_bounds(&region_range, lob, upb);
>              range_extend(range, &region_range);
>          }
>      }
> @@ -2467,7 +2468,7 @@ static void pci_dev_get_w64(PCIBus *b, PCIDevice *dev, 
> void *opaque)
>  
>  void pci_bus_get_w64_range(PCIBus *bus, Range *range)
>  {
> -    range->begin = range->end = 0;
> +    range_make_empty(range);
>      pci_for_each_device_under_bus(bus, pci_dev_get_w64, range);
>  }
>  
> diff --git a/include/qemu/range.h b/include/qemu/range.h
> index 3970f00..9296ba0 100644
> --- a/include/qemu/range.h
> +++ b/include/qemu/range.h
> @@ -30,18 +30,110 @@
>   *   - this can not represent a full 0 to ~0x0LL range.
>   */
>  
> +bool range_is_empty(Range *range);
> +bool range_contains(Range *range, uint64_t val);
> +void range_make_empty(Range *range);
> +void range_set_bounds(Range *range, uint64_t lob, uint64_t upb);
> +void range_set_bounds1(Range *range, uint64_t lob, uint64_t upb_plus1);
> +uint64_t range_lob(Range *range);
> +uint64_t range_upb(Range *range);
> +void range_extend(Range *range, Range *extend_by);
> +#ifdef RANGE_IMPL
> +
>  /* A structure representing a range of addresses. */
>  struct Range {
>      uint64_t begin; /* First byte of the range, or 0 if empty. */
>      uint64_t end;   /* 1 + the last byte. 0 if range empty or ends at 
> ~0x0LL. */
>  };
>  
> +static inline void range_invariant(Range *range)
> +{
> +    assert((!range->begin && !range->end) /* empty */
> +           || range->begin <= range->end - 1); /* non-empty */
> +}
> +
> +#define static
> +#define inline
> +
> +/* Compound literal encoding the empty range */
> +#define range_empty ((Range){ .begin = 0, .end = 0 })
> +
> +/* Is @range empty? */
> +static inline bool range_is_empty(Range *range)
> +{
> +    range_invariant(range);
> +    return !range->begin && !range->end;
> +}
> +
> +/* Does @range contain @val? */
> +static inline bool range_contains(Range *range, uint64_t val)
> +{
> +    return !range_is_empty(range)
> +        && val >= range->begin && val <= range->end - 1;
> +}
> +
> +/* Initialize @range to the empty range */
> +static inline void range_make_empty(Range *range)
> +{
> +    *range = range_empty;
> +    assert(range_is_empty(range));
> +}
> +
> +/*
> + * Initialize @range to span the interval address@hidden,@upb].
> + * Both bounds are inclusive.
> + * The interval must not be empty, i.e. @lob must be less than or
> + * equal @upb.
> + * The interval must not be [0,UINT64_MAX], because Range can't
> + * represent that.
> + */
> +static inline void range_set_bounds(Range *range, uint64_t lob, uint64_t upb)
> +{
> +    assert(lob <= upb);
> +    range->begin = lob;
> +    range->end = upb + 1;       /* may wrap to zero, that's okay */
> +    assert(!range_is_empty(range));
> +}
> +
> +/*
> + * Initialize @range to span the interval address@hidden,@upb_plus1).
> + * The lower bound is inclusive, the upper bound is exclusive.
> + * Zero @upb_plus1 is special: if @lob is also zero, set @range to the
> + * empty range.  Else, set @range to address@hidden,UINT64_MAX].
> + */
> +static inline void range_set_bounds1(Range *range,
> +                                     uint64_t lob, uint64_t upb_plus1)
> +{
> +    range->begin = lob;
> +    range->end = upb_plus1;
> +    range_invariant(range);
> +}
> +
> +/* Return @range's lower bound.  @range must not be empty. */
> +static inline uint64_t range_lob(Range *range)
> +{
> +    assert(!range_is_empty(range));
> +    return range->begin;
> +}
> +
> +/* Return @range's upper bound.  @range must not be empty. */
> +static inline uint64_t range_upb(Range *range)
> +{
> +    assert(!range_is_empty(range));
> +    return range->end - 1;
> +}
> +
> +/*
> + * Extend @range to the smallest interval that includes @extend_by, too.
> + * This must not extend @range to cover the interval [0,UINT64_MAX],
> + * because Range can't represent that.
> + */
>  static inline void range_extend(Range *range, Range *extend_by)
>  {
> -    if (!extend_by->begin && !extend_by->end) {
> +    if (range_is_empty(extend_by)) {
>          return;
>      }
> -    if (!range->begin && !range->end) {
> +    if (range_is_empty(range)) {
>          *range = *extend_by;
>          return;
>      }
> @@ -52,8 +144,18 @@ static inline void range_extend(Range *range, Range 
> *extend_by)
>      if (range->end - 1 < extend_by->end - 1) {
>          range->end = extend_by->end;
>      }
> +    /* Must not extend to { .begin = 0, .end = 0 }: */
> +    assert(!range_is_empty(range));
>  }
>  
> +#undef static
> +#undef inline
> +#else
> +struct Range {
> +    uint64_t begin_, end_;
> +};
> +#endif
> +
>  /* Get last byte of a range from offset + length.
>   * Undefined for ranges that wrap around 0. */
>  static inline uint64_t range_get_last(uint64_t offset, uint64_t len)
> diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
> index b546e5f..0690abb 100644
> --- a/qapi/string-input-visitor.c
> +++ b/qapi/string-input-visitor.c
> @@ -59,8 +59,7 @@ static int parse_str(StringInputVisitor *siv, const char 
> *name, Error **errp)
>          if (errno == 0 && endptr > str) {
>              if (*endptr == '\0') {
>                  cur = g_malloc0(sizeof(*cur));
> -                cur->begin = start;
> -                cur->end = start + 1;
> +                range_set_bounds(cur, start, start);
>                  siv->ranges = range_list_insert(siv->ranges, cur);
>                  cur = NULL;
>                  str = NULL;
> @@ -73,16 +72,14 @@ static int parse_str(StringInputVisitor *siv, const char 
> *name, Error **errp)
>                       end < start + 65536)) {
>                      if (*endptr == '\0') {
>                          cur = g_malloc0(sizeof(*cur));
> -                        cur->begin = start;
> -                        cur->end = end + 1;
> +                        range_set_bounds(cur, start, end);
>                          siv->ranges = range_list_insert(siv->ranges, cur);
>                          cur = NULL;
>                          str = NULL;
>                      } else if (*endptr == ',') {
>                          str = endptr + 1;
>                          cur = g_malloc0(sizeof(*cur));
> -                        cur->begin = start;
> -                        cur->end = end + 1;
> +                        range_set_bounds(cur, start, end);
>                          siv->ranges = range_list_insert(siv->ranges, cur);
>                          cur = NULL;
>                      } else {
> @@ -94,8 +91,7 @@ static int parse_str(StringInputVisitor *siv, const char 
> *name, Error **errp)
>              } else if (*endptr == ',') {
>                  str = endptr + 1;
>                  cur = g_malloc0(sizeof(*cur));
> -                cur->begin = start;
> -                cur->end = start + 1;
> +                range_set_bounds(cur, start, start);
>                  siv->ranges = range_list_insert(siv->ranges, cur);
>                  cur = NULL;
>              } else {
> @@ -134,7 +130,7 @@ start_list(Visitor *v, const char *name, GenericList 
> **list, size_t size,
>      if (siv->cur_range) {
>          Range *r = siv->cur_range->data;
>          if (r) {
> -            siv->cur = r->begin;
> +            siv->cur = range_lob(r);
>          }
>          *list = g_malloc0(size);
>      } else {
> @@ -156,7 +152,7 @@ static GenericList *next_list(Visitor *v, GenericList 
> *tail, size_t size)
>          return NULL;
>      }
>  
> -    if (siv->cur < r->begin || siv->cur >= r->end) {
> +    if (!range_contains(r, siv->cur)) {
>          siv->cur_range = g_list_next(siv->cur_range);
>          if (!siv->cur_range) {
>              return NULL;
> @@ -165,7 +161,7 @@ static GenericList *next_list(Visitor *v, GenericList 
> *tail, size_t size)
>          if (!r) {
>              return NULL;
>          }
> -        siv->cur = r->begin;
> +        siv->cur = range_lob(r);
>      }
>  
>      tail->next = g_malloc0(size);
> @@ -208,7 +204,7 @@ static void parse_type_int64(Visitor *v, const char 
> *name, int64_t *obj,
>              goto error;
>          }
>  
> -        siv->cur = r->begin;
> +        siv->cur = range_lob(r);
>      }
>  
>      *obj = siv->cur;
> diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
> index 5ea395a..c6f01f9 100644
> --- a/qapi/string-output-visitor.c
> +++ b/qapi/string-output-visitor.c
> @@ -83,8 +83,8 @@ static void string_output_set(StringOutputVisitor *sov, 
> char *string)
>  static void string_output_append(StringOutputVisitor *sov, int64_t a)
>  {
>      Range *r = g_malloc0(sizeof(*r));
> -    r->begin = a;
> -    r->end = a + 1;
> +
> +    range_set_bounds(r, a, a);
>      sov->ranges = range_list_insert(sov->ranges, r);
>  }
>  
> @@ -92,28 +92,28 @@ static void 
> string_output_append_range(StringOutputVisitor *sov,
>                                         int64_t s, int64_t e)
>  {
>      Range *r = g_malloc0(sizeof(*r));
> -    r->begin = s;
> -    r->end = e + 1;
> +
> +    range_set_bounds(r, s, e);
>      sov->ranges = range_list_insert(sov->ranges, r);
>  }
>  
>  static void format_string(StringOutputVisitor *sov, Range *r, bool next,
>                            bool human)
>  {
> -    if (r->end - r->begin > 1) {
> +    if (range_lob(r) != range_upb(r)) {
>          if (human) {
>              g_string_append_printf(sov->string, "0x%" PRIx64 "-0x%" PRIx64,
> -                                   r->begin, r->end - 1);
> +                                   range_lob(r), range_upb(r));
>  
>          } else {
>              g_string_append_printf(sov->string, "%" PRId64 "-%" PRId64,
> -                                   r->begin, r->end - 1);
> +                                   range_lob(r), range_upb(r));
>          }
>      } else {
>          if (human) {
> -            g_string_append_printf(sov->string, "0x%" PRIx64, r->begin);
> +            g_string_append_printf(sov->string, "0x%" PRIx64, range_lob(r));
>          } else {
> -            g_string_append_printf(sov->string, "%" PRId64, r->begin);
> +            g_string_append_printf(sov->string, "%" PRId64, range_lob(r));
>          }
>      }
>      if (next) {
> diff --git a/util/log.c b/util/log.c
> index f811d61..4da635c 100644
> --- a/util/log.c
> +++ b/util/log.c
> @@ -132,7 +132,7 @@ bool qemu_log_in_addr_range(uint64_t addr)
>          int i = 0;
>          for (i = 0; i < debug_regions->len; i++) {
>              Range *range = &g_array_index(debug_regions, Range, i);
> -            if (addr >= range->begin && addr <= range->end - 1) {
> +            if (range_contains(range, addr)) {
>                  return true;
>              }
>          }
> @@ -208,8 +208,7 @@ void qemu_set_dfilter_ranges(const char *filter_spec, 
> Error **errp)
>              error_setg(errp, "Invalid range");
>              goto out;
>          }
> -        range.begin = lob;
> -        range.end = upb + 1;
> +        range_set_bounds(&range, lob, upb);
>          g_array_append_val(debug_regions, range);
>      }
>  out:
> diff --git a/util/range.c b/util/range.c
> index 56e6baf..ab5102a 100644
> --- a/util/range.c
> +++ b/util/range.c
> @@ -19,6 +19,7 @@
>   */
>  
>  #include "qemu/osdep.h"
> +#define RANGE_IMPL
>  #include "qemu/range.h"
>  
>  /*
> @@ -46,8 +47,7 @@ GList *range_list_insert(GList *list, Range *data)
>  {
>      GList *l;
>  
> -    /* Range lists require no empty ranges */
> -    assert(data->begin < data->end || (data->begin && !data->end));
> +    assert(!range_is_empty(data));
>  
>      for (l = list; l && range_compare(l->data, data) < 0; l = l->next) {
>          /* Skip all list elements strictly less than data */
> -- 
> 2.5.5



reply via email to

[Prev in Thread] Current Thread [Next in Thread]