Re: [PATCH 1/9] Add cpu idle pwr mgmt to xen

[Keir Fraser <keir.fraser@eu.citrix.com>]

On 30/4/08 08:22, "Jan Beulich" <jbeulich@novell.com> wrote:

>>>> "Wei, Gang" <gang.wei@intel.com> 30.04.08 05:27 >>>
>> Revising done according to Jan's comments. Resend.
> 
> Thanks. Unfortunately you now use a static (but not per-CPU) variable -
> while I understand that it is expected that the call is done just once, I
> don't think this is a good thing to do.

Why is the variable even non-local? Is it just to make the xlat_malloc*()
interfaces simpler? It's a false simplification if so, and I think you'd be
better making the variable an explicit parameter to those functions.

Also I agree with Jan regarding non-ISO C usage of loop-header variable
declarations (don't do it) and also you should check copy_from_guest*()
return values and return -EFAULT where appropriate. His comment regarding
explicit padding or use of uint32_t in your public bitfield also sounds good
to me.

 -- Keir