On 03/11/2014 06:53 AM, arei.gong...@huawei.com wrote: > From: ChenLiang <chenlian...@huawei.com> >
The subject line says what, but there is no WHY. The commit message
should explain why you want a signed type.
> Signed-off-by: ChenLiang <chenlian...@huawei.com>
> Signed-off-by: Gonglei <arei.gong...@huawei.com>
> ---
> arch_init.c | 4 ++--
> include/migration/page_cache.h | 4 ++--
> page_cache.c | 6 +++---
> 3 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index 461a10a..1c1488a 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -109,7 +109,7 @@ const uint32_t arch_type = QEMU_ARCH;
> static bool mig_throttle_on;
> static void check_guest_throttling(void);
>
> -static uint64_t bitmap_sync_cnt;
> +static int64_t bitmap_sync_cnt;
Wait. Patch 2/10 introduced this variable. Why not introduce it with
the correct type to begin with? Please reorder your series so that you
aren't churning on new code.
> -static inline uint64_t get_bitmap_sync_cnt(void)
> +static inline int64_t get_bitmap_sync_cnt(void)
> {
> return bitmap_sync_cnt;
Not to mention that my argument in patch 2 still holds - unless you are
going to export this function, it is pointless compared to just using
the variable directly.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
