Message ID | 1283164293-11820-1-git-send-email-Jes.Sorensen@redhat.com |
---|---|
State | New |
Headers | show |
Jes.Sorensen@redhat.com wrote: > From: Jes Sorensen <Jes.Sorensen@redhat.com> > > There is no need to check for dest < 0 or vector >= 0 as both are > uint16_t. > > This should fix problems with broken build with aggressive compiler > flags. Reported by Xudong Hao <xudong.hao@intel.com> > > Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com> > --- > hw/ivshmem.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/ivshmem.c b/hw/ivshmem.c > index bbb5cba..afebbc3 100644 > --- a/hw/ivshmem.c > +++ b/hw/ivshmem.c > @@ -199,13 +199,13 @@ static void ivshmem_io_writel(void *opaque, > target_phys_addr_t addr, > > case DOORBELL: > /* check that dest VM ID is reasonable */ > - if ((dest < 0) || (dest > s->max_peer)) { > + if (dest > s->max_peer) { > IVSHMEM_DPRINTF("Invalid destination VM ID (%d)\n", > dest); break; > } > > /* check doorbell range */ > - if ((vector >= 0) && (vector < > s->peers[dest].nb_eventfds)) { + if (vector < > s->peers[dest].nb_eventfds) { > > IVSHMEM_DPRINTF("Writing %" PRId64 " to VM %d on > vector %d\n", write_one, dest, vector); if > (write(s->peers[dest].eventfds[vector], This patch works for me. Thanks, Xudong
Hao, Xudong wrote: > Jes.Sorensen@redhat.com wrote: >> From: Jes Sorensen <Jes.Sorensen@redhat.com> >> >> There is no need to check for dest < 0 or vector >= 0 as both are >> uint16_t. >> >> This should fix problems with broken build with aggressive compiler >> flags. Reported by Xudong Hao <xudong.hao@intel.com> >> >> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com> --- >> hw/ivshmem.c | 4 ++-- >> 1 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/hw/ivshmem.c b/hw/ivshmem.c >> index bbb5cba..afebbc3 100644 >> --- a/hw/ivshmem.c >> +++ b/hw/ivshmem.c >> @@ -199,13 +199,13 @@ static void ivshmem_io_writel(void *opaque, >> target_phys_addr_t addr, >> >> case DOORBELL: >> /* check that dest VM ID is reasonable */ >> - if ((dest < 0) || (dest > s->max_peer)) { >> + if (dest > s->max_peer) { >> IVSHMEM_DPRINTF("Invalid destination VM ID (%d)\n", >> dest); break; >> } >> >> /* check doorbell range */ >> - if ((vector >= 0) && (vector < >> s->peers[dest].nb_eventfds)) { + if (vector < >> s->peers[dest].nb_eventfds) { >> >> IVSHMEM_DPRINTF("Writing %" PRId64 " to VM %d on >> vector %d\n", write_one, dest, vector); if >> (write(s->peers[dest].eventfds[vector], > > This patch works for me. > Jes, correct result is this patch works for me on x86_64 system. However, in i386 system, there is another bug: ... CC x86_64-softmmu/ivshmem.o CC x86_64-softmmu/fpu/softfloat-native.o CC x86_64-softmmu/op_helper.o cc1: warnings being treated as errors /home/build/gitrepo/qemu/hw/ivshmem.c: In function 'check_shm_size. /home/build/gitrepo/qemu/hw/ivshmem.c:357: warning: format '%ld' expects type 'long int', but argt 5 has type '__off64_t make[1]: *** [ivshmem.o] Error 1 make[1]: *** Waiting for unfinished jobs.... make: *** [subdir-x86_64-softmmu] Error 2 Thanks, Xudong
On Mon, Aug 30, 2010 at 4:31 AM, <Jes.Sorensen@redhat.com> wrote: > From: Jes Sorensen <Jes.Sorensen@redhat.com> > > There is no need to check for dest < 0 or vector >= 0 as both are > uint16_t. > > This should fix problems with broken build with aggressive compiler > flags. Reported by Xudong Hao <xudong.hao@intel.com> > > Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com> > --- > hw/ivshmem.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/ivshmem.c b/hw/ivshmem.c > index bbb5cba..afebbc3 100644 > --- a/hw/ivshmem.c > +++ b/hw/ivshmem.c > @@ -199,13 +199,13 @@ static void ivshmem_io_writel(void *opaque, target_phys_addr_t addr, > > case DOORBELL: > /* check that dest VM ID is reasonable */ > - if ((dest < 0) || (dest > s->max_peer)) { > + if (dest > s->max_peer) { > IVSHMEM_DPRINTF("Invalid destination VM ID (%d)\n", dest); > break; > } > > /* check doorbell range */ > - if ((vector >= 0) && (vector < s->peers[dest].nb_eventfds)) { > + if (vector < s->peers[dest].nb_eventfds) { > IVSHMEM_DPRINTF("Writing %" PRId64 " to VM %d on vector %d\n", > write_one, dest, vector); > if (write(s->peers[dest].eventfds[vector], > -- > 1.7.2.2 > > Acked-by: Cam Macdonell <cam@cs.ualberta.ca> I sent a patch yesterday that addressed it, but it is probably better to remove the tests that switched to signed 16-bit ints. Anthony, please apply this one. Cam
On Tue, Aug 31, 2010 at 6:51 PM, Hao, Xudong <xudong.hao@intel.com> wrote: > Hao, Xudong wrote: >> Jes.Sorensen@redhat.com wrote: >>> From: Jes Sorensen <Jes.Sorensen@redhat.com> >>> >>> There is no need to check for dest < 0 or vector >= 0 as both are >>> uint16_t. >>> >>> This should fix problems with broken build with aggressive compiler >>> flags. Reported by Xudong Hao <xudong.hao@intel.com> >>> >>> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com> --- >>> hw/ivshmem.c | 4 ++-- >>> 1 files changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/hw/ivshmem.c b/hw/ivshmem.c >>> index bbb5cba..afebbc3 100644 >>> --- a/hw/ivshmem.c >>> +++ b/hw/ivshmem.c >>> @@ -199,13 +199,13 @@ static void ivshmem_io_writel(void *opaque, >>> target_phys_addr_t addr, >>> >>> case DOORBELL: >>> /* check that dest VM ID is reasonable */ >>> - if ((dest < 0) || (dest > s->max_peer)) { >>> + if (dest > s->max_peer) { >>> IVSHMEM_DPRINTF("Invalid destination VM ID (%d)\n", >>> dest); break; >>> } >>> >>> /* check doorbell range */ >>> - if ((vector >= 0) && (vector < >>> s->peers[dest].nb_eventfds)) { + if (vector < >>> s->peers[dest].nb_eventfds) { >>> >>> IVSHMEM_DPRINTF("Writing %" PRId64 " to VM %d on >>> vector %d\n", write_one, dest, vector); if >>> (write(s->peers[dest].eventfds[vector], >> >> This patch works for me. >> > > Jes, correct result is this patch works for me on x86_64 system. However, in i386 system, there is another bug: > > ... > CC x86_64-softmmu/ivshmem.o > CC x86_64-softmmu/fpu/softfloat-native.o > CC x86_64-softmmu/op_helper.o > cc1: warnings being treated as errors > /home/build/gitrepo/qemu/hw/ivshmem.c: In function 'check_shm_size. > /home/build/gitrepo/qemu/hw/ivshmem.c:357: warning: format '%ld' expects type 'long int', but argt 5 has type '__off64_t > make[1]: *** [ivshmem.o] Error 1 > make[1]: *** Waiting for unfinished jobs.... > make: *** [subdir-x86_64-softmmu] Error 2 Hmm, that was causing problems on 32-bit systems, not 64-bit. Is this with gcc 4.1.2? Please the try the following patch from Avi that should add the necessary cast. http://www.mail-archive.com/qemu-devel@nongnu.org/msg40715.html Cam
On Tue, Aug 31, 2010 at 9:56 PM, Cam Macdonell <cam@cs.ualberta.ca> wrote: > On Tue, Aug 31, 2010 at 6:51 PM, Hao, Xudong <xudong.hao@intel.com> wrote: >> Hao, Xudong wrote: >>> Jes.Sorensen@redhat.com wrote: >>>> From: Jes Sorensen <Jes.Sorensen@redhat.com> >>>> >>>> There is no need to check for dest < 0 or vector >= 0 as both are >>>> uint16_t. >>>> >>>> This should fix problems with broken build with aggressive compiler >>>> flags. Reported by Xudong Hao <xudong.hao@intel.com> >>>> >>>> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com> --- >>>> hw/ivshmem.c | 4 ++-- >>>> 1 files changed, 2 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/hw/ivshmem.c b/hw/ivshmem.c >>>> index bbb5cba..afebbc3 100644 >>>> --- a/hw/ivshmem.c >>>> +++ b/hw/ivshmem.c >>>> @@ -199,13 +199,13 @@ static void ivshmem_io_writel(void *opaque, >>>> target_phys_addr_t addr, >>>> >>>> case DOORBELL: >>>> /* check that dest VM ID is reasonable */ >>>> - if ((dest < 0) || (dest > s->max_peer)) { >>>> + if (dest > s->max_peer) { >>>> IVSHMEM_DPRINTF("Invalid destination VM ID (%d)\n", >>>> dest); break; >>>> } >>>> >>>> /* check doorbell range */ >>>> - if ((vector >= 0) && (vector < >>>> s->peers[dest].nb_eventfds)) { + if (vector < >>>> s->peers[dest].nb_eventfds) { >>>> >>>> IVSHMEM_DPRINTF("Writing %" PRId64 " to VM %d on >>>> vector %d\n", write_one, dest, vector); if >>>> (write(s->peers[dest].eventfds[vector], >>> >>> This patch works for me. >>> >> >> Jes, correct result is this patch works for me on x86_64 system. However, in i386 system, there is another bug: >> >> ... >> CC x86_64-softmmu/ivshmem.o >> CC x86_64-softmmu/fpu/softfloat-native.o >> CC x86_64-softmmu/op_helper.o >> cc1: warnings being treated as errors >> /home/build/gitrepo/qemu/hw/ivshmem.c: In function 'check_shm_size. >> /home/build/gitrepo/qemu/hw/ivshmem.c:357: warning: format '%ld' expects type 'long int', but argt 5 has type '__off64_t >> make[1]: *** [ivshmem.o] Error 1 >> make[1]: *** Waiting for unfinished jobs.... >> make: *** [subdir-x86_64-softmmu] Error 2 > > Hmm, that was causing problems on 32-bit systems, not 64-bit. Is this > with gcc 4.1.2? Oh sorry, I read too fast, you did say it was on 32-bit. Avi's patch will do the trick. Cam
Thanks, applied. On Mon, Aug 30, 2010 at 10:31 AM, <Jes.Sorensen@redhat.com> wrote: > From: Jes Sorensen <Jes.Sorensen@redhat.com> > > There is no need to check for dest < 0 or vector >= 0 as both are > uint16_t. > > This should fix problems with broken build with aggressive compiler > flags. Reported by Xudong Hao <xudong.hao@intel.com> > > Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com> > --- > hw/ivshmem.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/ivshmem.c b/hw/ivshmem.c > index bbb5cba..afebbc3 100644 > --- a/hw/ivshmem.c > +++ b/hw/ivshmem.c > @@ -199,13 +199,13 @@ static void ivshmem_io_writel(void *opaque, target_phys_addr_t addr, > > case DOORBELL: > /* check that dest VM ID is reasonable */ > - if ((dest < 0) || (dest > s->max_peer)) { > + if (dest > s->max_peer) { > IVSHMEM_DPRINTF("Invalid destination VM ID (%d)\n", dest); > break; > } > > /* check doorbell range */ > - if ((vector >= 0) && (vector < s->peers[dest].nb_eventfds)) { > + if (vector < s->peers[dest].nb_eventfds) { > IVSHMEM_DPRINTF("Writing %" PRId64 " to VM %d on vector %d\n", > write_one, dest, vector); > if (write(s->peers[dest].eventfds[vector], > -- > 1.7.2.2 > > >
diff --git a/hw/ivshmem.c b/hw/ivshmem.c index bbb5cba..afebbc3 100644 --- a/hw/ivshmem.c +++ b/hw/ivshmem.c @@ -199,13 +199,13 @@ static void ivshmem_io_writel(void *opaque, target_phys_addr_t addr, case DOORBELL: /* check that dest VM ID is reasonable */ - if ((dest < 0) || (dest > s->max_peer)) { + if (dest > s->max_peer) { IVSHMEM_DPRINTF("Invalid destination VM ID (%d)\n", dest); break; } /* check doorbell range */ - if ((vector >= 0) && (vector < s->peers[dest].nb_eventfds)) { + if (vector < s->peers[dest].nb_eventfds) { IVSHMEM_DPRINTF("Writing %" PRId64 " to VM %d on vector %d\n", write_one, dest, vector); if (write(s->peers[dest].eventfds[vector],