Re: [Xen-devel] [PATCH v2 2/9] x86/intel_pstate: add some calculation related support

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.05.16 at 09:49, <wei.w.wang@xxxxxxxxx> wrote:
> The added calculation related functions will be used in the intel_pstate.c.

If these are taken from Linux, please say so here (including the version
they got cloned from), as in that case close review can be considered
unnecessary. That said - do you really need all of them, considering
that you don't need to support a 32-bit environment?


> --- a/xen/include/asm-x86/div64.h
> +++ b/xen/include/asm-x86/div64.h

All changes to this file that are indeed needed should follow Xen
coding style.

> +/*
> + * abs() handles unsigned and signed longs, ints, shorts and chars. For all
> + * input types abs() returns a signed long.
> + * abs() should not be used for 64-bit types (s64, u64, long long) - use 
> abs64()
> + * for those.
> + */
> +#define abs(x) ({                      \
> +    long ret;                          \
> +    if (sizeof(x) == sizeof(long)) {   \
> +        long __x = (x);                \
> +        ret = (__x < 0) ? -__x : __x;  \
> +    } else {                           \
> +        int __x = (x);                 \
> +        ret = (__x < 0) ? -__x : __x;  \
> +    }                                  \
> +    ret;                               \
> +})

Please use the already present and more generic (less the unsigned
types support, which I highly question to be necessary here) ABS()
instead.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel