[Qemu-devel] [PATCH v2] Fix compilation of check-qint.c by using a long long integer constant

[Qemu-devel] [PATCH v2] Fix compilation of check-qint.c by using a long long integer constant

[Pierre Riteau]

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;
 
     qi = qint_from_int(value);
     fail_unless(qi->value == value);
-- 
1.6.3.3

Re: [Qemu-devel] [PATCH v2] Fix compilation of check-qint.c by using a long long integer constant

[Reimar Döffinger]

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?)?

Re: [Qemu-devel] [PATCH v2] Fix compilation of check-qint.c by using a long long integer constant

[Luiz Capitulino]

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.

Re: [Qemu-devel] [PATCH v2] Fix compilation of check-qint.c by using a long long integer constant

[malc]

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.

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [PATCH v2] Fix compilation of check-qint.c by using a long long integer constant

[Pierre Riteau]

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?

-- 
Pierre Riteau -- http://perso.univ-rennes1.fr/pierre.riteau/

Re: [Qemu-devel] [PATCH v2] Fix compilation of check-qint.c by using a long long integer constant

[Luiz Capitulino]

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.

Re: [Qemu-devel] [PATCH v2] Fix compilation of check-qint.c by using a long long integer constant

[Jamie Lokier]

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

Re: [Qemu-devel] [PATCH v2] Fix compilation of check-qint.c by using a long long integer constant

[Reimar Döffinger]

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.

Re: [Qemu-devel] [PATCH v2] Fix compilation of check-qint.c by using a long long integer constant

[Pierre Riteau]

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.

-- 
Pierre Riteau -- http://perso.univ-rennes1.fr/pierre.riteau/