| Message ID | 1252084198-15375-1-git-send-email-Pierre.Riteau@irisa.fr |
|---|---|
| State | Superseded |
| Headers | show |
On Fri, Sep 04, 2009 at 07:09:58PM +0200, Pierre Riteau wrote: > Error was: > check-qint.c:46: error: integer constant is too large for 'long' type > --- > check-qint.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/check-qint.c b/check-qint.c > index ae5d22f..f5c054e 100644 > --- a/check-qint.c > +++ b/check-qint.c > @@ -43,7 +43,7 @@ END_TEST > START_TEST(qint_from_int64_test) > { > QInt *qi; > - const int64_t value = 0xffffffffffffffff; > + const int64_t value = 0xffffffffffffffffLL; Hm, well it does not really fit in a signed long long either (so from that aspect it should be ULL). Should it not be simply -1 (does qemu assume all architectures use two's complement?)?
On Fri, 4 Sep 2009 19:09:58 +0200 Pierre Riteau <Pierre.Riteau@irisa.fr> wrote: > Error was: > check-qint.c:46: error: integer constant is too large for 'long' type You forgot the Signed-off-by line, otherwise: Acked-by: Luiz Capitulino <lcapitulino@redhat.com> I've built the series on i386, but forgot to enable the test-suite.
On Fri, 4 Sep 2009, Reimar D?ffinger wrote: > On Fri, Sep 04, 2009 at 07:09:58PM +0200, Pierre Riteau wrote: > > Error was: > > check-qint.c:46: error: integer constant is too large for 'long' type > > --- > > check-qint.c | 2 +- > > 1 files changed, 1 insertions(+), 1 deletions(-) > > > > diff --git a/check-qint.c b/check-qint.c > > index ae5d22f..f5c054e 100644 > > --- a/check-qint.c > > +++ b/check-qint.c > > @@ -43,7 +43,7 @@ END_TEST > > START_TEST(qint_from_int64_test) > > { > > QInt *qi; > > - const int64_t value = 0xffffffffffffffff; > > + const int64_t value = 0xffffffffffffffffLL; > > Hm, well it does not really fit in a signed long long either (so from that > aspect it should be ULL). > Should it not be simply -1 (does qemu assume all architectures > use two's complement?)? Yes.
On 4 sept. 09, at 19:29, Reimar Döffinger wrote: > On Fri, Sep 04, 2009 at 07:09:58PM +0200, Pierre Riteau wrote: >> Error was: >> check-qint.c:46: error: integer constant is too large for 'long' type >> --- >> check-qint.c | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/check-qint.c b/check-qint.c >> index ae5d22f..f5c054e 100644 >> --- a/check-qint.c >> +++ b/check-qint.c >> @@ -43,7 +43,7 @@ END_TEST >> START_TEST(qint_from_int64_test) >> { >> QInt *qi; >> - const int64_t value = 0xffffffffffffffff; >> + const int64_t value = 0xffffffffffffffffLL; > > Hm, well it does not really fit in a signed long long either (so > from that > aspect it should be ULL). > Should it not be simply -1 (does qemu assume all architectures > use two's complement?)? How about INT64_MIN?
On Fri, 4 Sep 2009 19:56:30 +0200 Pierre Riteau <Pierre.Riteau@irisa.fr> wrote: > > On 4 sept. 09, at 19:29, Reimar Döffinger wrote: > > > On Fri, Sep 04, 2009 at 07:09:58PM +0200, Pierre Riteau wrote: > >> Error was: > >> check-qint.c:46: error: integer constant is too large for 'long' type > >> --- > >> check-qint.c | 2 +- > >> 1 files changed, 1 insertions(+), 1 deletions(-) > >> > >> diff --git a/check-qint.c b/check-qint.c > >> index ae5d22f..f5c054e 100644 > >> --- a/check-qint.c > >> +++ b/check-qint.c > >> @@ -43,7 +43,7 @@ END_TEST > >> START_TEST(qint_from_int64_test) > >> { > >> QInt *qi; > >> - const int64_t value = 0xffffffffffffffff; > >> + const int64_t value = 0xffffffffffffffffLL; > > > > Hm, well it does not really fit in a signed long long either (so > > from that > > aspect it should be ULL). > > Should it not be simply -1 (does qemu assume all architectures > > use two's complement?)? > > > How about INT64_MIN? The test is there to assure that QInt can handle integers > 32-bits, so it's not an issue if we have fewer bits.
malc wrote: > > > - const int64_t value = 0xffffffffffffffff; > > > + const int64_t value = 0xffffffffffffffffLL; > > > > Hm, well it does not really fit in a signed long long either (so from that > > aspect it should be ULL). > > Should it not be simply -1 (does qemu assume all architectures > > use two's complement?)? > > Yes. Yes, but be aware that GCC nowadays does some optimisations which assume calculations do not wraparound when using signed types. See -fwrapv and -fno-strict-overflow. They tripped up the Linux kernel recently, causing some range tests to be optimised away. Those optimisations warrant caution when thinking in two's complement while using signed types. For unsigned types, the ANSI C language uses arithmetic (mod 2^bits) for unsigned types. You can always write 0ULL-1, even theoretically on non-two's complement machines: that's well-defined in C to have all one bits set. -- Jamie
On Fri, Sep 04, 2009 at 03:25:20PM -0300, Luiz Capitulino wrote: > On Fri, 4 Sep 2009 19:56:30 +0200 > Pierre Riteau <Pierre.Riteau@irisa.fr> wrote: > > > > > On 4 sept. 09, at 19:29, Reimar Döffinger wrote: > > > > > On Fri, Sep 04, 2009 at 07:09:58PM +0200, Pierre Riteau wrote: > > >> Error was: > > >> check-qint.c:46: error: integer constant is too large for 'long' type > > >> --- > > >> check-qint.c | 2 +- > > >> 1 files changed, 1 insertions(+), 1 deletions(-) > > >> > > >> diff --git a/check-qint.c b/check-qint.c > > >> index ae5d22f..f5c054e 100644 > > >> --- a/check-qint.c > > >> +++ b/check-qint.c > > >> @@ -43,7 +43,7 @@ END_TEST > > >> START_TEST(qint_from_int64_test) > > >> { > > >> QInt *qi; > > >> - const int64_t value = 0xffffffffffffffff; > > >> + const int64_t value = 0xffffffffffffffffLL; > > > > > > Hm, well it does not really fit in a signed long long either (so > > > from that > > > aspect it should be ULL). > > > Should it not be simply -1 (does qemu assume all architectures > > > use two's complement?)? > > > > > > How about INT64_MIN? > > The test is there to assure that QInt can handle > integers > 32-bits, so it's not an issue if we have > fewer bits. For that this value is nonsense, this test could succeed even with a 32 bit QInt and sign expansion. Not to mention that also a conversion to float and back would keep the value unchanged, although it would lose information in the general case. If it's supposed to test that it will really store 64 bit it should be some random number with variation in both upper and lower bits.
On 5 sept. 2009, at 10:07, Reimar Döffinger wrote: > On Fri, Sep 04, 2009 at 03:25:20PM -0300, Luiz Capitulino wrote: >> On Fri, 4 Sep 2009 19:56:30 +0200 >> Pierre Riteau <Pierre.Riteau@irisa.fr> wrote: >> >>> >>> On 4 sept. 09, at 19:29, Reimar Döffinger wrote: >>> >>>> On Fri, Sep 04, 2009 at 07:09:58PM +0200, Pierre Riteau wrote: >>>>> Error was: >>>>> check-qint.c:46: error: integer constant is too large for 'long' >>>>> type >>>>> --- >>>>> check-qint.c | 2 +- >>>>> 1 files changed, 1 insertions(+), 1 deletions(-) >>>>> >>>>> diff --git a/check-qint.c b/check-qint.c >>>>> index ae5d22f..f5c054e 100644 >>>>> --- a/check-qint.c >>>>> +++ b/check-qint.c >>>>> @@ -43,7 +43,7 @@ END_TEST >>>>> START_TEST(qint_from_int64_test) >>>>> { >>>>> QInt *qi; >>>>> - const int64_t value = 0xffffffffffffffff; >>>>> + const int64_t value = 0xffffffffffffffffLL; >>>> >>>> Hm, well it does not really fit in a signed long long either (so >>>> from that >>>> aspect it should be ULL). >>>> Should it not be simply -1 (does qemu assume all architectures >>>> use two's complement?)? >>> >>> >>> How about INT64_MIN? >> >> The test is there to assure that QInt can handle >> integers > 32-bits, so it's not an issue if we have >> fewer bits. > > For that this value is nonsense, this test could succeed even with a > 32 > bit QInt and sign expansion. > Not to mention that also a conversion to float and back would keep the > value unchanged, although it would lose information in the general > case. > If it's supposed to test that it will really store 64 bit it should be > some random number with variation in both upper and lower bits. > > Very good point. I sent a new patch to the list.
diff --git a/check-qint.c b/check-qint.c index ae5d22f..f5c054e 100644 --- a/check-qint.c +++ b/check-qint.c @@ -43,7 +43,7 @@ END_TEST START_TEST(qint_from_int64_test) { QInt *qi; - const int64_t value = 0xffffffffffffffff; + const int64_t value = 0xffffffffffffffffLL; qi = qint_from_int(value); fail_unless(qi->value == value);