On Thu, 30 Aug 2012 16:21:31 +0300 "Michael S. Tsirkin" <m...@redhat.com> wrote: > > +static u32 apic_read_reg(int reg_off, void *bitmap) > > +{ > > + return *((u32 *)(bitmap + reg_off)); > > +} > > + > > Contrast with apic_set_reg which gets apic, > add fact that all callers invoke REG_POS and you will > see this is a bad API. > > I played with some APIs but in the end it's > probably better to just open-code this.
I don't mind open-coding this. > As a bonus, open-coding will avoid the need > for cast above, which is good: casts make code more > fragile. But I still don't understand why we can eliminate casting: u32 reg_val; reg_val = *((u32 *)(bitmap + REG_POS(vec))); if (reg_val) return __fls(reg_val) + vec; (I'm not sure compilers are allowed to push out the value and do multiple references for this code as explained in https://lwn.net/Articles/508991/ ) If you mean u32 *reg; reg = bitmap + REG_POS(vec); if (*reg) return __fls(*reg) + vec; I'm still not confident if this is a good style. I rarely see code doing if (*p) __fls(*p); This looks like explicite multiple references: I'm not saying this will actually be compiled to do multiple references. Thanks, Takuya -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html