* [Qemu-devel] fix unsigned comparison warning in TCG @ 2010-09-26 17:40 Hollis Blanchard 2010-09-26 18:33 ` Blue Swirl 0 siblings, 1 reply; 10+ messages in thread From: Hollis Blanchard @ 2010-09-26 17:40 UTC (permalink / raw) To: qemu-devel TCGOpcode is an enum, which apparently can be unsigned. Signed-off-by: Hollis Blanchard <hollis@penguinppc.org> --- % ./configure --target-list=ppcemb-softmmu --enable-debug % make ... CC ppcemb-softmmu/tcg/tcg.o cc1: warnings being treated as errors /home/hollisb/source/qemu.git/tcg/tcg.c: In function ‘tcg_add_target_add_op_defs’: /home/hollisb/source/qemu.git/tcg/tcg.c:1030: error: comparison of unsigned expression >= 0 is always true % gcc -v gcc version 4.4.4 20100630 (Red Hat 4.4.4-10) (GCC) diff --git a/tcg/tcg.c b/tcg/tcg.c index e0a9030..7e96859 100644 --- a/tcg/tcg.c +++ b/tcg/tcg.c @@ -1027,7 +1027,7 @@ void tcg_add_target_add_op_defs(const TCGTargetOpDef *tdef if (tdefs->op == (TCGOpcode)-1) break; op = tdefs->op; - assert(op >= 0 && op < NB_OPS); + assert(op < NB_OPS); def = &tcg_op_defs[op]; #if defined(CONFIG_DEBUG_TCG) /* Duplicate entry in op definitions? */ ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] fix unsigned comparison warning in TCG 2010-09-26 17:40 [Qemu-devel] fix unsigned comparison warning in TCG Hollis Blanchard @ 2010-09-26 18:33 ` Blue Swirl 2010-10-07 23:15 ` Venkateswararao Jujjuri (JV) 2010-10-08 8:32 ` [Qemu-devel] [PATCH] tcg: Fix compiler error (comparison of unsigned expression) Stefan Weil 0 siblings, 2 replies; 10+ messages in thread From: Blue Swirl @ 2010-09-26 18:33 UTC (permalink / raw) To: Hollis Blanchard; +Cc: qemu-devel On Sun, Sep 26, 2010 at 5:40 PM, Hollis Blanchard <hollis@penguinppc.org> wrote: > TCGOpcode is an enum, which apparently can be unsigned. > > Signed-off-by: Hollis Blanchard <hollis@penguinppc.org> > --- > > % ./configure --target-list=ppcemb-softmmu --enable-debug > % make > ... > CC ppcemb-softmmu/tcg/tcg.o > cc1: warnings being treated as errors > /home/hollisb/source/qemu.git/tcg/tcg.c: In function > ‘tcg_add_target_add_op_defs’: > /home/hollisb/source/qemu.git/tcg/tcg.c:1030: error: comparison of > unsigned expression >= 0 is always true > % gcc -v > gcc version 4.4.4 20100630 (Red Hat 4.4.4-10) (GCC) > > diff --git a/tcg/tcg.c b/tcg/tcg.c > index e0a9030..7e96859 100644 > --- a/tcg/tcg.c > +++ b/tcg/tcg.c > @@ -1027,7 +1027,7 @@ void tcg_add_target_add_op_defs(const TCGTargetOpDef *tdef > if (tdefs->op == (TCGOpcode)-1) > break; > op = tdefs->op; > - assert(op >= 0 && op < NB_OPS); > + assert(op < NB_OPS); Please add int cast, like 95ee3914bfd551aeec49932a400530141865acad. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] fix unsigned comparison warning in TCG 2010-09-26 18:33 ` Blue Swirl @ 2010-10-07 23:15 ` Venkateswararao Jujjuri (JV) 2010-10-08 8:32 ` [Qemu-devel] [PATCH] tcg: Fix compiler error (comparison of unsigned expression) Stefan Weil 1 sibling, 0 replies; 10+ messages in thread From: Venkateswararao Jujjuri (JV) @ 2010-10-07 23:15 UTC (permalink / raw) To: Blue Swirl; +Cc: qemu-devel, Hollis Blanchard On 9/26/2010 11:33 AM, Blue Swirl wrote: > On Sun, Sep 26, 2010 at 5:40 PM, Hollis Blanchard <hollis@penguinppc.org> wrote: >> TCGOpcode is an enum, which apparently can be unsigned. >> >> Signed-off-by: Hollis Blanchard <hollis@penguinppc.org> >> --- >> >> % ./configure --target-list=ppcemb-softmmu --enable-debug >> % make >> ... >> CC ppcemb-softmmu/tcg/tcg.o >> cc1: warnings being treated as errors >> /home/hollisb/source/qemu.git/tcg/tcg.c: In function >> ‘tcg_add_target_add_op_defs’: >> /home/hollisb/source/qemu.git/tcg/tcg.c:1030: error: comparison of >> unsigned expression >= 0 is always true >> % gcc -v >> gcc version 4.4.4 20100630 (Red Hat 4.4.4-10) (GCC) >> >> diff --git a/tcg/tcg.c b/tcg/tcg.c >> index e0a9030..7e96859 100644 >> --- a/tcg/tcg.c >> +++ b/tcg/tcg.c >> @@ -1027,7 +1027,7 @@ void tcg_add_target_add_op_defs(const TCGTargetOpDef *tdef >> if (tdefs->op == (TCGOpcode)-1) >> break; >> op = tdefs->op; >> - assert(op >= 0 && op < NB_OPS); >> + assert(op < NB_OPS); > > Please add int cast, like 95ee3914bfd551aeec49932a400530141865acad. > What is the latest on this? Waiting for this fix as I use --enable-debug a lot. :) - JV ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH] tcg: Fix compiler error (comparison of unsigned expression) 2010-09-26 18:33 ` Blue Swirl 2010-10-07 23:15 ` Venkateswararao Jujjuri (JV) @ 2010-10-08 8:32 ` Stefan Weil 2010-10-08 16:57 ` [Qemu-devel] " Hollis Blanchard 2010-10-20 20:56 ` Blue Swirl 1 sibling, 2 replies; 10+ messages in thread From: Stefan Weil @ 2010-10-08 8:32 UTC (permalink / raw) To: QEMU Developers; +Cc: Blue Swirl, Hollis Blanchard When qemu is configured with --enable-debug-tcg, gcc throws this warning (or error with -Werror): tcg/tcg.c:1030: error: comparison of unsigned expression >= 0 is always true Fix it by removing the >= 0 part. The type cast to 'unsigned' catches negative values of op (which should never happen). This is a modification of Hollis Blanchard's patch. Cc: Hollis Blanchard <hollis@penguinppc.org> Cc: Blue Swirl <blauwirbel@gmail.com> Signed-off-by: Stefan Weil <weil@mail.berlios.de> --- tcg/tcg.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/tcg/tcg.c b/tcg/tcg.c index e0a9030..0cdef0d 100644 --- a/tcg/tcg.c +++ b/tcg/tcg.c @@ -1027,7 +1027,7 @@ void tcg_add_target_add_op_defs(const TCGTargetOpDef *tdefs) if (tdefs->op == (TCGOpcode)-1) break; op = tdefs->op; - assert(op >= 0 && op < NB_OPS); + assert((unsigned)op < NB_OPS); def = &tcg_op_defs[op]; #if defined(CONFIG_DEBUG_TCG) /* Duplicate entry in op definitions? */ -- 1.7.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] Re: [PATCH] tcg: Fix compiler error (comparison of unsigned expression) 2010-10-08 8:32 ` [Qemu-devel] [PATCH] tcg: Fix compiler error (comparison of unsigned expression) Stefan Weil @ 2010-10-08 16:57 ` Hollis Blanchard 2010-10-08 21:43 ` Stefan Weil 2010-10-20 20:56 ` Blue Swirl 1 sibling, 1 reply; 10+ messages in thread From: Hollis Blanchard @ 2010-10-08 16:57 UTC (permalink / raw) To: Stefan Weil; +Cc: Blue Swirl, QEMU Developers On Fri, Oct 8, 2010 at 1:32 AM, Stefan Weil <weil@mail.berlios.de> wrote: > When qemu is configured with --enable-debug-tcg, > gcc throws this warning (or error with -Werror): > > tcg/tcg.c:1030: error: comparison of unsigned expression >= 0 is always true > > Fix it by removing the >= 0 part. > The type cast to 'unsigned' catches negative values of op > (which should never happen). > > This is a modification of Hollis Blanchard's patch. > > Cc: Hollis Blanchard <hollis@penguinppc.org> > Cc: Blue Swirl <blauwirbel@gmail.com> > Signed-off-by: Stefan Weil <weil@mail.berlios.de> > --- > tcg/tcg.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/tcg/tcg.c b/tcg/tcg.c > index e0a9030..0cdef0d 100644 > --- a/tcg/tcg.c > +++ b/tcg/tcg.c > @@ -1027,7 +1027,7 @@ void tcg_add_target_add_op_defs(const TCGTargetOpDef *tdefs) > if (tdefs->op == (TCGOpcode)-1) > break; > op = tdefs->op; > - assert(op >= 0 && op < NB_OPS); > + assert((unsigned)op < NB_OPS); > def = &tcg_op_defs[op]; > #if defined(CONFIG_DEBUG_TCG) > /* Duplicate entry in op definitions? */ According to the warning, op is already unsigned, so this simply removes the >=0 test, which was my original patch. In contrast, Blue wanted a cast to int, as seen in 95ee3914bfd551aeec49932a400530141865acad. -Hollis ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] Re: [PATCH] tcg: Fix compiler error (comparison of unsigned expression) 2010-10-08 16:57 ` [Qemu-devel] " Hollis Blanchard @ 2010-10-08 21:43 ` Stefan Weil 2010-10-13 18:58 ` Stefan Weil 0 siblings, 1 reply; 10+ messages in thread From: Stefan Weil @ 2010-10-08 21:43 UTC (permalink / raw) To: Hollis Blanchard; +Cc: Blue Swirl, QEMU Developers Am 08.10.2010 18:57, schrieb Hollis Blanchard: > On Fri, Oct 8, 2010 at 1:32 AM, Stefan Weil <weil@mail.berlios.de> wrote: >> When qemu is configured with --enable-debug-tcg, >> gcc throws this warning (or error with -Werror): >> >> tcg/tcg.c:1030: error: comparison of unsigned expression >= 0 is >> always true >> >> Fix it by removing the >= 0 part. >> The type cast to 'unsigned' catches negative values of op >> (which should never happen). >> >> This is a modification of Hollis Blanchard's patch. >> >> Cc: Hollis Blanchard <hollis@penguinppc.org> >> Cc: Blue Swirl <blauwirbel@gmail.com> >> Signed-off-by: Stefan Weil <weil@mail.berlios.de> >> --- >> tcg/tcg.c | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/tcg/tcg.c b/tcg/tcg.c >> index e0a9030..0cdef0d 100644 >> --- a/tcg/tcg.c >> +++ b/tcg/tcg.c >> @@ -1027,7 +1027,7 @@ void tcg_add_target_add_op_defs(const >> TCGTargetOpDef *tdefs) >> if (tdefs->op == (TCGOpcode)-1) >> break; >> op = tdefs->op; >> - assert(op >= 0 && op < NB_OPS); >> + assert((unsigned)op < NB_OPS); >> def = &tcg_op_defs[op]; >> #if defined(CONFIG_DEBUG_TCG) >> /* Duplicate entry in op definitions? */ > > According to the warning, op is already unsigned, so this simply > removes the >=0 test, which was my original patch. > > In contrast, Blue wanted a cast to int, as seen in > 95ee3914bfd551aeec49932a400530141865acad. > > -Hollis I read the original thread. Michael already proposed the unsigned cast in that thread. Your patch is correct as long as we use gcc and as long as gcc uses unsigned enums when possible (it is possible here). Other compilers or new versions of gcc might use signed enums. For those (and also for current gcc versions), my patch works. Blue's solution also works but is a bit more code without having advantages. There are even more solutions which are accepted by the compiler: ((op > first && op <= last) || (op == first)) with first, last being the first or last enum member. - Stefan ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] Re: [PATCH] tcg: Fix compiler error (comparison of unsigned expression) 2010-10-08 21:43 ` Stefan Weil @ 2010-10-13 18:58 ` Stefan Weil 2010-10-13 19:22 ` Hollis Blanchard 0 siblings, 1 reply; 10+ messages in thread From: Stefan Weil @ 2010-10-13 18:58 UTC (permalink / raw) To: Hollis Blanchard; +Cc: Blue Swirl, QEMU Developers Am 08.10.2010 23:43, schrieb Stefan Weil: > Am 08.10.2010 18:57, schrieb Hollis Blanchard: >> On Fri, Oct 8, 2010 at 1:32 AM, Stefan Weil <weil@mail.berlios.de> >> wrote: >>> When qemu is configured with --enable-debug-tcg, >>> gcc throws this warning (or error with -Werror): >>> >>> tcg/tcg.c:1030: error: comparison of unsigned expression >= 0 is >>> always true >>> >>> Fix it by removing the >= 0 part. >>> The type cast to 'unsigned' catches negative values of op >>> (which should never happen). >>> >>> This is a modification of Hollis Blanchard's patch. >>> >>> Cc: Hollis Blanchard <hollis@penguinppc.org> >>> Cc: Blue Swirl <blauwirbel@gmail.com> >>> Signed-off-by: Stefan Weil <weil@mail.berlios.de> >>> --- >>> tcg/tcg.c | 2 +- >>> 1 files changed, 1 insertions(+), 1 deletions(-) >>> >>> diff --git a/tcg/tcg.c b/tcg/tcg.c >>> index e0a9030..0cdef0d 100644 >>> --- a/tcg/tcg.c >>> +++ b/tcg/tcg.c >>> @@ -1027,7 +1027,7 @@ void tcg_add_target_add_op_defs(const >>> TCGTargetOpDef *tdefs) >>> if (tdefs->op == (TCGOpcode)-1) >>> break; >>> op = tdefs->op; >>> - assert(op >= 0 && op < NB_OPS); >>> + assert((unsigned)op < NB_OPS); >>> def = &tcg_op_defs[op]; >>> #if defined(CONFIG_DEBUG_TCG) >>> /* Duplicate entry in op definitions? */ >> >> According to the warning, op is already unsigned, so this simply >> removes the >=0 test, which was my original patch. >> >> In contrast, Blue wanted a cast to int, as seen in >> 95ee3914bfd551aeec49932a400530141865acad. >> >> -Hollis > > I read the original thread. Michael already proposed the > unsigned cast in that thread. > > Your patch is correct as long as we use gcc and as long > as gcc uses unsigned enums when possible (it is possible here). > > Other compilers or new versions of gcc might use signed enums. > For those (and also for current gcc versions), my patch works. > Blue's solution also works but is a bit more code without > having advantages. > > There are even more solutions which are accepted by the > compiler: ((op > first && op <= last) || (op == first)) > with first, last being the first or last enum member. > > - Stefan Hollis, do you still see problems with my patch? Or can it be committed? Regards, Stefan ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] Re: [PATCH] tcg: Fix compiler error (comparison of unsigned expression) 2010-10-13 18:58 ` Stefan Weil @ 2010-10-13 19:22 ` Hollis Blanchard 2010-10-13 19:33 ` Blue Swirl 0 siblings, 1 reply; 10+ messages in thread From: Hollis Blanchard @ 2010-10-13 19:22 UTC (permalink / raw) To: Stefan Weil; +Cc: Blue Swirl, QEMU Developers On Wed, Oct 13, 2010 at 11:58 AM, Stefan Weil <weil@mail.berlios.de> wrote: > > Hollis, do you still see problems with my patch? > Or can it be committed? I have no objection; I was just anticipating Blue's objection when I commented previously. It's just a style question really... -Hollis ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] Re: [PATCH] tcg: Fix compiler error (comparison of unsigned expression) 2010-10-13 19:22 ` Hollis Blanchard @ 2010-10-13 19:33 ` Blue Swirl 0 siblings, 0 replies; 10+ messages in thread From: Blue Swirl @ 2010-10-13 19:33 UTC (permalink / raw) To: Hollis Blanchard; +Cc: QEMU Developers On Wed, Oct 13, 2010 at 7:22 PM, Hollis Blanchard <hollis@penguinppc.org> wrote: > On Wed, Oct 13, 2010 at 11:58 AM, Stefan Weil <weil@mail.berlios.de> wrote: >> >> Hollis, do you still see problems with my patch? >> Or can it be committed? > > I have no objection; I was just anticipating Blue's objection when I > commented previously. It's just a style question really... Both ((int)op >= 0 && op < NB_OPS) and ((unsigned int)op < NB_OPS) produce same binary (if NB_OPS < MAX_INT). During the earlier, long discussion, the int version was preferred. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] Re: [PATCH] tcg: Fix compiler error (comparison of unsigned expression) 2010-10-08 8:32 ` [Qemu-devel] [PATCH] tcg: Fix compiler error (comparison of unsigned expression) Stefan Weil 2010-10-08 16:57 ` [Qemu-devel] " Hollis Blanchard @ 2010-10-20 20:56 ` Blue Swirl 1 sibling, 0 replies; 10+ messages in thread From: Blue Swirl @ 2010-10-20 20:56 UTC (permalink / raw) To: Stefan Weil; +Cc: QEMU Developers, Hollis Blanchard Thanks, applied. On Fri, Oct 8, 2010 at 8:32 AM, Stefan Weil <weil@mail.berlios.de> wrote: > When qemu is configured with --enable-debug-tcg, > gcc throws this warning (or error with -Werror): > > tcg/tcg.c:1030: error: comparison of unsigned expression >= 0 is always true > > Fix it by removing the >= 0 part. > The type cast to 'unsigned' catches negative values of op > (which should never happen). > > This is a modification of Hollis Blanchard's patch. > > Cc: Hollis Blanchard <hollis@penguinppc.org> > Cc: Blue Swirl <blauwirbel@gmail.com> > Signed-off-by: Stefan Weil <weil@mail.berlios.de> > --- > tcg/tcg.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/tcg/tcg.c b/tcg/tcg.c > index e0a9030..0cdef0d 100644 > --- a/tcg/tcg.c > +++ b/tcg/tcg.c > @@ -1027,7 +1027,7 @@ void tcg_add_target_add_op_defs(const TCGTargetOpDef *tdefs) > if (tdefs->op == (TCGOpcode)-1) > break; > op = tdefs->op; > - assert(op >= 0 && op < NB_OPS); > + assert((unsigned)op < NB_OPS); > def = &tcg_op_defs[op]; > #if defined(CONFIG_DEBUG_TCG) > /* Duplicate entry in op definitions? */ > -- > 1.7.1 > > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2010-10-20 20:56 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-09-26 17:40 [Qemu-devel] fix unsigned comparison warning in TCG Hollis Blanchard 2010-09-26 18:33 ` Blue Swirl 2010-10-07 23:15 ` Venkateswararao Jujjuri (JV) 2010-10-08 8:32 ` [Qemu-devel] [PATCH] tcg: Fix compiler error (comparison of unsigned expression) Stefan Weil 2010-10-08 16:57 ` [Qemu-devel] " Hollis Blanchard 2010-10-08 21:43 ` Stefan Weil 2010-10-13 18:58 ` Stefan Weil 2010-10-13 19:22 ` Hollis Blanchard 2010-10-13 19:33 ` Blue Swirl 2010-10-20 20:56 ` Blue Swirl
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).