diff mbox

[v2] kvm: Fix enable_cap helpers on older gcc

Message ID 1399888297-4751-1-git-send-email-agraf@suse.de
State New
Headers show

Commit Message

Alexander Graf May 12, 2014, 9:51 a.m. UTC
Commit 40f1ee27aa1 introduced handy helpers for enable_cap calls on
vcpu and vm level. Unfortunately some older gcc versions (4.7.1, 4.6)
seem to choke on signedness detection in inline created variables:

target-ppc/kvm.c: In function 'kvmppc_booke_watchdog_enable':
target-ppc/kvm.c:1302:21: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
target-ppc/kvm.c: In function 'kvmppc_set_papr':
target-ppc/kvm.c:1504:21: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]

However - thanks to Thomas Huth for the suggestion - we can just cast the
offending potentially 0 value to a signed type, making the comparison signed.

Signed-off-by: Alexander Graf <agraf@suse.de>

---

v1 -> v2:

  - only do cast, leaving readability intact

---
 include/sysemu/kvm.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Thomas Huth May 12, 2014, 9:56 a.m. UTC | #1
On Mon, 12 May 2014 11:51:37 +0200
Alexander Graf <agraf@suse.de> wrote:

> Commit 40f1ee27aa1 introduced handy helpers for enable_cap calls on
> vcpu and vm level. Unfortunately some older gcc versions (4.7.1, 4.6)
> seem to choke on signedness detection in inline created variables:
> 
> target-ppc/kvm.c: In function 'kvmppc_booke_watchdog_enable':
> target-ppc/kvm.c:1302:21: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
> target-ppc/kvm.c: In function 'kvmppc_set_papr':
> target-ppc/kvm.c:1504:21: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
> 
> However - thanks to Thomas Huth for the suggestion - we can just cast the
> offending potentially 0 value to a signed type, making the comparison signed.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> 
> ---
> 
> v1 -> v2:
> 
>   - only do cast, leaving readability intact
> 
> ---
>  include/sysemu/kvm.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index 192fe89..76c39cc 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -302,7 +302,7 @@ int kvm_check_extension(KVMState *s, unsigned int extension);
>          };                                                           \
>          uint64_t args_tmp[] = { __VA_ARGS__ };                       \
>          int i;                                                       \
> -        for (i = 0; i < ARRAY_SIZE(args_tmp) &&                      \
> +        for (i = 0; i < (int)ARRAY_SIZE(args_tmp) &&                 \
>                       i < ARRAY_SIZE(cap.args); i++) {                \
>              cap.args[i] = args_tmp[i];                               \
>          }                                                            \
> @@ -317,7 +317,7 @@ int kvm_check_extension(KVMState *s, unsigned int extension);
>          };                                                           \
>          uint64_t args_tmp[] = { __VA_ARGS__ };                       \
>          int i;                                                       \
> -        for (i = 0; i < ARRAY_SIZE(args_tmp) &&                      \
> +        for (i = 0; i < (int)ARRAY_SIZE(args_tmp) &&                 \
>                       i < ARRAY_SIZE(cap.args); i++) {                \
>              cap.args[i] = args_tmp[i];                               \
>          }                                                            \

Looks much better this way :-)

Reviewed-by: Thomas Huth <thuth@linux.vnet.ibm.com>
Cornelia Huck May 12, 2014, 10:15 a.m. UTC | #2
On Mon, 12 May 2014 11:51:37 +0200
Alexander Graf <agraf@suse.de> wrote:

> Commit 40f1ee27aa1 introduced handy helpers for enable_cap calls on
> vcpu and vm level. Unfortunately some older gcc versions (4.7.1, 4.6)
> seem to choke on signedness detection in inline created variables:
> 
> target-ppc/kvm.c: In function 'kvmppc_booke_watchdog_enable':
> target-ppc/kvm.c:1302:21: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
> target-ppc/kvm.c: In function 'kvmppc_set_papr':
> target-ppc/kvm.c:1504:21: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
> 
> However - thanks to Thomas Huth for the suggestion - we can just cast the
> offending potentially 0 value to a signed type, making the comparison signed.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>

Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>

I assume you will take this through your tree?

> 
> ---
> 
> v1 -> v2:
> 
>   - only do cast, leaving readability intact
> 
> ---
>  include/sysemu/kvm.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index 192fe89..76c39cc 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -302,7 +302,7 @@ int kvm_check_extension(KVMState *s, unsigned int extension);
>          };                                                           \
>          uint64_t args_tmp[] = { __VA_ARGS__ };                       \
>          int i;                                                       \
> -        for (i = 0; i < ARRAY_SIZE(args_tmp) &&                      \
> +        for (i = 0; i < (int)ARRAY_SIZE(args_tmp) &&                 \
>                       i < ARRAY_SIZE(cap.args); i++) {                \
>              cap.args[i] = args_tmp[i];                               \
>          }                                                            \
> @@ -317,7 +317,7 @@ int kvm_check_extension(KVMState *s, unsigned int extension);
>          };                                                           \
>          uint64_t args_tmp[] = { __VA_ARGS__ };                       \
>          int i;                                                       \
> -        for (i = 0; i < ARRAY_SIZE(args_tmp) &&                      \
> +        for (i = 0; i < (int)ARRAY_SIZE(args_tmp) &&                 \
>                       i < ARRAY_SIZE(cap.args); i++) {                \
>              cap.args[i] = args_tmp[i];                               \
>          }                                                            \
Alexander Graf May 12, 2014, 10:17 a.m. UTC | #3
On 12.05.14 12:15, Cornelia Huck wrote:
> On Mon, 12 May 2014 11:51:37 +0200
> Alexander Graf <agraf@suse.de> wrote:
>
>> Commit 40f1ee27aa1 introduced handy helpers for enable_cap calls on
>> vcpu and vm level. Unfortunately some older gcc versions (4.7.1, 4.6)
>> seem to choke on signedness detection in inline created variables:
>>
>> target-ppc/kvm.c: In function 'kvmppc_booke_watchdog_enable':
>> target-ppc/kvm.c:1302:21: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
>> target-ppc/kvm.c: In function 'kvmppc_set_papr':
>> target-ppc/kvm.c:1504:21: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
>>
>> However - thanks to Thomas Huth for the suggestion - we can just cast the
>> offending potentially 0 value to a signed type, making the comparison signed.
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
> Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
>
> I assume you will take this through your tree?

Yeah, without I can't compile :)


Alex
Cornelia Huck May 20, 2014, 10:53 a.m. UTC | #4
On Mon, 12 May 2014 11:51:37 +0200
Alexander Graf <agraf@suse.de> wrote:

> Commit 40f1ee27aa1 introduced handy helpers for enable_cap calls on
> vcpu and vm level. Unfortunately some older gcc versions (4.7.1, 4.6)
> seem to choke on signedness detection in inline created variables:
> 
> target-ppc/kvm.c: In function 'kvmppc_booke_watchdog_enable':
> target-ppc/kvm.c:1302:21: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
> target-ppc/kvm.c: In function 'kvmppc_set_papr':
> target-ppc/kvm.c:1504:21: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
> 
> However - thanks to Thomas Huth for the suggestion - we can just cast the
> offending potentially 0 value to a signed type, making the comparison signed.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> 

Mind if I take this through s390-next instead of your tree, so I can
send a pull request with clean conscience?
Alexander Graf May 20, 2014, 10:53 a.m. UTC | #5
On 20.05.14 12:53, Cornelia Huck wrote:
> On Mon, 12 May 2014 11:51:37 +0200
> Alexander Graf <agraf@suse.de> wrote:
>
>> Commit 40f1ee27aa1 introduced handy helpers for enable_cap calls on
>> vcpu and vm level. Unfortunately some older gcc versions (4.7.1, 4.6)
>> seem to choke on signedness detection in inline created variables:
>>
>> target-ppc/kvm.c: In function 'kvmppc_booke_watchdog_enable':
>> target-ppc/kvm.c:1302:21: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
>> target-ppc/kvm.c: In function 'kvmppc_set_papr':
>> target-ppc/kvm.c:1504:21: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
>>
>> However - thanks to Thomas Huth for the suggestion - we can just cast the
>> offending potentially 0 value to a signed type, making the comparison signed.
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>
> Mind if I take this through s390-next instead of your tree, so I can
> send a pull request with clean conscience?
>

Sure, whoever hits master first wins ;)


Alex
diff mbox

Patch

diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 192fe89..76c39cc 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -302,7 +302,7 @@  int kvm_check_extension(KVMState *s, unsigned int extension);
         };                                                           \
         uint64_t args_tmp[] = { __VA_ARGS__ };                       \
         int i;                                                       \
-        for (i = 0; i < ARRAY_SIZE(args_tmp) &&                      \
+        for (i = 0; i < (int)ARRAY_SIZE(args_tmp) &&                 \
                      i < ARRAY_SIZE(cap.args); i++) {                \
             cap.args[i] = args_tmp[i];                               \
         }                                                            \
@@ -317,7 +317,7 @@  int kvm_check_extension(KVMState *s, unsigned int extension);
         };                                                           \
         uint64_t args_tmp[] = { __VA_ARGS__ };                       \
         int i;                                                       \
-        for (i = 0; i < ARRAY_SIZE(args_tmp) &&                      \
+        for (i = 0; i < (int)ARRAY_SIZE(args_tmp) &&                 \
                      i < ARRAY_SIZE(cap.args); i++) {                \
             cap.args[i] = args_tmp[i];                               \
         }                                                            \