On Thu, 30 Aug 2012 16:21:31 +0300
"Michael S. Tsirkin" <mst@xxxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
