Re: [Qemu-devel] [QEMU PATCH 2/7] hw/apic.c: rename bit functions to not conflict with bitops.h

[Eduardo Habkost]

On Thu, Jul 12, 2012 at 07:24:35PM +0000, Blue Swirl wrote:
> On Tue, Jul 10, 2012 at 8:22 PM, Eduardo Habkost <address@hidden> wrote:
> > Signed-off-by: Eduardo Habkost <address@hidden>
> 
> Maybe the bitops functions should be renamed instead, for example
> prefixed by 'qemu_'. That may be safer if one day the kernel find
> their way to system headers too.

Well, if there's any risk the kernel functions will conflict with the
QEMU function names, that would be an additional reason to rename the
apic.c functions too, so they don't conflict with the kernel functions
either.

Personally, I would never sent a patch to rename the bitops.h functions,
as the current names work perfectly to me.

> 
> > ---
> >  hw/apic.c |   34 +++++++++++++++++-----------------
> >  1 file changed, 17 insertions(+), 17 deletions(-)
> >
> > diff --git a/hw/apic.c b/hw/apic.c
> > index 60552df..d322fe3 100644
> > --- a/hw/apic.c
> > +++ b/hw/apic.c
> > @@ -50,7 +50,7 @@ static int ffs_bit(uint32_t value)
> >      return ctz32(value);
> >  }
> >
> > -static inline void set_bit(uint32_t *tab, int index)
> > +static inline void apic_set_bit(uint32_t *tab, int index)
> >  {
> >      int i, mask;
> >      i = index >> 5;
> > @@ -58,7 +58,7 @@ static inline void set_bit(uint32_t *tab, int index)
> >      tab[i] |= mask;
> >  }
> >
> > -static inline void reset_bit(uint32_t *tab, int index)
> > +static inline void apic_reset_bit(uint32_t *tab, int index)
> >  {
> >      int i, mask;
> >      i = index >> 5;
> > @@ -66,7 +66,7 @@ static inline void reset_bit(uint32_t *tab, int index)
> >      tab[i] &= ~mask;
> >  }
> >
> > -static inline int get_bit(uint32_t *tab, int index)
> > +static inline int apic_get_bit(uint32_t *tab, int index)
> >  {
> >      int i, mask;
> >      i = index >> 5;
> > @@ -183,7 +183,7 @@ void apic_deliver_pic_intr(DeviceState *d, int level)
> >          case APIC_DM_FIXED:
> >              if (!(lvt & APIC_LVT_LEVEL_TRIGGER))
> >                  break;
> > -            reset_bit(s->irr, lvt & 0xff);
> > +            apic_reset_bit(s->irr, lvt & 0xff);
> >              /* fall through */
> >          case APIC_DM_EXTINT:
> >              cpu_reset_interrupt(s->cpu_env, CPU_INTERRUPT_HARD);
> > @@ -379,13 +379,13 @@ void apic_poll_irq(DeviceState *d)
> >
> >  static void apic_set_irq(APICCommonState *s, int vector_num, int 
> > trigger_mode)
> >  {
> > -    apic_report_irq_delivered(!get_bit(s->irr, vector_num));
> > +    apic_report_irq_delivered(!apic_get_bit(s->irr, vector_num));
> >
> > -    set_bit(s->irr, vector_num);
> > +    apic_set_bit(s->irr, vector_num);
> >      if (trigger_mode)
> > -        set_bit(s->tmr, vector_num);
> > +        apic_set_bit(s->tmr, vector_num);
> >      else
> > -        reset_bit(s->tmr, vector_num);
> > +        apic_reset_bit(s->tmr, vector_num);
> >      if (s->vapic_paddr) {
> >          apic_sync_vapic(s, SYNC_ISR_IRR_TO_VAPIC);
> >          /*
> > @@ -405,8 +405,8 @@ static void apic_eoi(APICCommonState *s)
> >      isrv = get_highest_priority_int(s->isr);
> >      if (isrv < 0)
> >          return;
> > -    reset_bit(s->isr, isrv);
> > -    if (!(s->spurious_vec & APIC_SV_DIRECTED_IO) && get_bit(s->tmr, isrv)) 
> > {
> > +    apic_reset_bit(s->isr, isrv);
> > +    if (!(s->spurious_vec & APIC_SV_DIRECTED_IO) && apic_get_bit(s->tmr, 
> > isrv)) {
> >          ioapic_eoi_broadcast(isrv);
> >      }
> >      apic_sync_vapic(s, SYNC_FROM_VAPIC | SYNC_TO_VAPIC);
> > @@ -445,7 +445,7 @@ static void apic_get_delivery_bitmask(uint32_t 
> > *deliver_bitmask,
> >              int idx = apic_find_dest(dest);
> >              memset(deliver_bitmask, 0x00, MAX_APIC_WORDS * 
> > sizeof(uint32_t));
> >              if (idx >= 0)
> > -                set_bit(deliver_bitmask, idx);
> > +                apic_set_bit(deliver_bitmask, idx);
> >          }
> >      } else {
> >          /* XXX: cluster mode */
> > @@ -455,11 +455,11 @@ static void apic_get_delivery_bitmask(uint32_t 
> > *deliver_bitmask,
> >              if (apic_iter) {
> >                  if (apic_iter->dest_mode == 0xf) {
> >                      if (dest & apic_iter->log_dest)
> > -                        set_bit(deliver_bitmask, i);
> > +                        apic_set_bit(deliver_bitmask, i);
> >                  } else if (apic_iter->dest_mode == 0x0) {
> >                      if ((dest & 0xf0) == (apic_iter->log_dest & 0xf0) &&
> >                          (dest & apic_iter->log_dest & 0x0f)) {
> > -                        set_bit(deliver_bitmask, i);
> > +                        apic_set_bit(deliver_bitmask, i);
> >                      }
> >                  }
> >              } else {
> > @@ -502,14 +502,14 @@ static void apic_deliver(DeviceState *d, uint8_t 
> > dest, uint8_t dest_mode,
> >          break;
> >      case 1:
> >          memset(deliver_bitmask, 0x00, sizeof(deliver_bitmask));
> > -        set_bit(deliver_bitmask, s->idx);
> > +        apic_set_bit(deliver_bitmask, s->idx);
> >          break;
> >      case 2:
> >          memset(deliver_bitmask, 0xff, sizeof(deliver_bitmask));
> >          break;
> >      case 3:
> >          memset(deliver_bitmask, 0xff, sizeof(deliver_bitmask));
> > -        reset_bit(deliver_bitmask, s->idx);
> > +        apic_reset_bit(deliver_bitmask, s->idx);
> >          break;
> >      }
> >
> > @@ -557,8 +557,8 @@ int apic_get_interrupt(DeviceState *d)
> >          apic_sync_vapic(s, SYNC_TO_VAPIC);
> >          return s->spurious_vec & 0xff;
> >      }
> > -    reset_bit(s->irr, intno);
> > -    set_bit(s->isr, intno);
> > +    apic_reset_bit(s->irr, intno);
> > +    apic_set_bit(s->isr, intno);
> >      apic_sync_vapic(s, SYNC_TO_VAPIC);
> >      apic_update_irq(s);
> >      return intno;
> > --
> > 1.7.10.4
> >
> >
> 

-- 
Eduardo