Stephan von Krawczynski <[email protected]> writes:
> during tests with latest SuSE distro 8.2 compiling 2.4.21-pre6 showed a lot of
> "comparison between signed and unsigned" warnings. It looks like SuSE ships gcc
I also noticed lots of compiler warnings with gcc 3.3, now default in SuSE,
and cleaned up most of them for the cifs vfs but there are a few that just
look wrong for gcc to spit out warnings on. For example the following
local variable definition and the similar ones in the same file
(fs/cifs/inode.c):
__u64 uid = 0xFFFFFFFFFFFFFFFF;
generates a warning saying the value is too long for a long on
x86 SuSE 8.2 with gcc 3.3 - which makes no sense. Any value
above 0xFFFFFFFFF generates the same warning (intuitively
36 bits should fit in an unsigned 64 bit local variable).
Defining the literal with the UL suffix didn't seem to help - and I
rebelled against the solutions that work for this case ie casting
the local variable which is already __u64 to __u64 but that
presumably would work for those three, as would a (__u64)cast
of -1 which seems equally ugly). Unfortunately for the CIFS
Unix Extensions these values really are supposed to be 64 bit
on the wire (0xFFFFFFFFFFFFFFFF indicating ignore setting this value).
In the current version of the cifs vfs (http://cifs.bkbits.net/linux-2.5cifs)
the only other case that now generates compiler warnings (in this case
signed/unsigned compares) is the comparison of unsigned local variables
to the literal PAGE_CACHE_SIZE (which presumably is interpreted as
signed by the compiler).
Any idea what is going on in this weird gcc 3.3 behavior where it thinks
64 bits can't fit in a __u64 local variable?
Steve French <[email protected]> writes:
> look wrong for gcc to spit out warnings on. For example the following
> local variable definition and the similar ones in the same file
> (fs/cifs/inode.c):
>
> __u64 uid = 0xFFFFFFFFFFFFFFFF;
>
> generates a warning saying the value is too long for a long on x86
> SuSE 8.2 with gcc 3.3 - which makes no sense. Any value
> above 0xFFFFFFFFF generates the same warning (intuitively
> 36 bits should fit in an unsigned 64 bit local variable).
>
> Defining the literal with the UL suffix didn't seem to help - and I
Define it with ULL (= long long)
> Any idea what is going on in this weird gcc 3.3 behavior where it thinks
> 64 bits can't fit in a __u64 local variable? -
AFAIK the problem is that it has no default promotion for constants to
long long (normally they are int, long, unsigned long etc. depending on
their value) It's some C99 thing. Or maybe a gcc bug. Anyways ULL
makes it clear that it is unsigned long long.
-Andi
P.S.: The warning is thankfully turned off by default again in later
compilers.
On Wed, 11 Jun 2003, Steve French wrote:
> Stephan von Krawczynski <[email protected]> writes:
>
> > during tests with latest SuSE distro 8.2 compiling 2.4.21-pre6 showed a lot of
> > "comparison between signed and unsigned" warnings. It looks like SuSE ships gcc
>
> I also noticed lots of compiler warnings with gcc 3.3, now default in SuSE,
> and cleaned up most of them for the cifs vfs but there are a few that just
> look wrong for gcc to spit out warnings on. For example the following
> local variable definition and the similar ones in the same file
> (fs/cifs/inode.c):
>
> __u64 uid = 0xFFFFFFFFFFFFFFFF;
>
> generates a warning saying the value is too long for a long on
> x86 SuSE 8.2 with gcc 3.3 - which makes no sense. Any value
> above 0xFFFFFFFFF generates the same warning (intuitively
> 36 bits should fit in an unsigned 64 bit local variable).
[SNIPPED...]
I think the compiler doesn't have a default type for something
that long. Therefore you have to define is as:
__u64 uid = 0xFFFFFFFFFFFFFFFFULL;
Seems dumb, but it even works.
Cheers,
Dick Johnson
Penguin : Linux version 2.4.20 on an i686 machine (797.90 BogoMips).
Why is the government concerned about the lunatic fringe? Think about it.
On Wed, 11 Jun 2003, Steve French wrote:
> Stephan von Krawczynski <[email protected]> writes:
>
> >during tests with latest SuSE distro 8.2 compiling 2.4.21-pre6 showed a
> >lot of
> >"comparison between signed and unsigned" warnings. It looks like SuSE
> >ships gcc
>
> I also noticed lots of compiler warnings with gcc 3.3, now default in SuSE,
> and cleaned up most of them for the cifs vfs but there are a few that just
> look wrong for gcc to spit out warnings on. For example the following
> local variable definition and the similar ones in the same file
> (fs/cifs/inode.c):
Did you try with a release version of gcc 3.3? The one shipped on SuSE
Linux 8.2 media/FTP is a pre-release version.
(If SuSE would only ship patch-level updates to such essential
components, say, 3.2.2 for 8.1 and the released 3.3 for 8.2.)
--
Matthias Andree
On Wed, 11 Jun 2003, Richard B. Johnson wrote:
> On Wed, 11 Jun 2003, Steve French wrote:
>
> > Stephan von Krawczynski <[email protected]> writes:
> >
> > > during tests with latest SuSE distro 8.2 compiling 2.4.21-pre6 showed a lot of
> > > "comparison between signed and unsigned" warnings. It looks like SuSE ships gcc
> >
> > I also noticed lots of compiler warnings with gcc 3.3, now default in SuSE,
> > and cleaned up most of them for the cifs vfs but there are a few that just
> > look wrong for gcc to spit out warnings on. For example the following
> > local variable definition and the similar ones in the same file
> > (fs/cifs/inode.c):
> >
> > __u64 uid = 0xFFFFFFFFFFFFFFFF;
> >
> > generates a warning saying the value is too long for a long on
> > x86 SuSE 8.2 with gcc 3.3 - which makes no sense. Any value
> > above 0xFFFFFFFFF generates the same warning (intuitively
> > 36 bits should fit in an unsigned 64 bit local variable).
> [SNIPPED...]
>
> I think the compiler doesn't have a default type for something
> that long. Therefore you have to define is as:
>
> __u64 uid = 0xFFFFFFFFFFFFFFFFULL;
>
> Seems dumb, but it even works.
>
FYI some Spanish-language mailer is set up for an automatic response.
Please don't auto-respond to a mail-list!
Cheers,
Dick Johnson
Penguin : Linux version 2.4.20 on an i686 machine (797.90 BogoMips).
Why is the government concerned about the lunatic fringe? Think about it.
Steve French <[email protected]> writes:
|> I also noticed lots of compiler warnings with gcc 3.3, now default in
|> SuSE, and cleaned up most of them for the cifs vfs but there are a few
|> that just
|> look wrong for gcc to spit out warnings on. For example the following
|> local variable definition and the similar ones in the same file
|> (fs/cifs/inode.c):
|>
|> __u64 uid = 0xFFFFFFFFFFFFFFFF;
|>
|> generates a warning saying the value is too long for a long on x86 SuSE
|> 8.2 with gcc 3.3 - which makes no sense. Any value
|> above 0xFFFFFFFFF generates the same warning (intuitively
|> 36 bits should fit in an unsigned 64 bit local variable).
An expression is evaluated independent of the context, ie. the fact that
the left side of the assignment is a 64 bit variable has no significance
at all. But I agree that the warning should only occur in c89 mode, not
in the default gnu89 mode, where long long is available. And in fact the
generated code will be correct.
|> Defining the literal with the UL suffix didn't seem to help
Yes, since you need a long long literal.
|> rebelled against the solutions that work for this case ie casting the
|> local variable which is already __u64 to __u64 but that presumably would
|> work for those three, as would a (__u64)cast of -1 which seems equally
|> ugly).
You can just use -1. The implicit conversion to __u64 will DTRT.
Andreas.
--
Andreas Schwab, SuSE Labs, [email protected]
SuSE Linux AG, Deutschherrnstr. 15-19, D-90429 N?rnberg
Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
Although it fixes it for building on 32 bit architectures, won't changing
__u64 uid = 0xFFFFFFFFFFFFFFFF;
to
__u64 uid = 0xFFFFFFFFFFFFFFFFULL;
generate a type mismatch warning on ppc64 and similar 64 bit
architecutres since __u64 is not a unsigned long long on ppc64
(it is unsigned long)? My gut reaction is to just ingore the three
places that cause warnings and the remaining two places that cause
signed/unsigned compare warnings of unsigned int local variables
to #defined literals (which presumably are treated as signed by default).
Andi Kleen wrote:
>Steve French <[email protected]> writes:
>
>>... and the similar ones in the same file
>>(fs/cifs/inode.c):
>>
>> __u64 uid = 0xFFFFFFFFFFFFFFFF;
>>
>>generates a warning saying the value is too long for a long on x86
>>SuSE 8.2 with gcc 3.3
>>
>
>Define it with ULL (= long long)
>
>
>AFAIK the problem is that it has no default promotion for constants to
>long long (normally they are int, long, unsigned long etc. depending on
>their value) It's some C99 thing. Or maybe a gcc bug. Anyways ULL
>makes it clear that it is unsigned long long.
>
Steve French <[email protected]> wrote:
>
> Although it fixes it for building on 32 bit architectures, won't changing
>
>
> __u64 uid = 0xFFFFFFFFFFFFFFFF;
> to
>
> __u64 uid = 0xFFFFFFFFFFFFFFFFULL;
>
> generate a type mismatch warning on ppc64 and similar 64 bit
> architecutres since __u64 is not a unsigned long long on ppc64
> (it is unsigned long)?
u64 uid = -1;
will work just nicely.
In article <[email protected]>,
Steve French <[email protected]> wrote:
>Although it fixes it for building on 32 bit architectures, won't changing
>
>
> __u64 uid = 0xFFFFFFFFFFFFFFFF;
>to
>
> __u64 uid = 0xFFFFFFFFFFFFFFFFULL;
>
>generate a type mismatch warning on ppc64 and similar 64 bit
>architecutres since __u64 is not a unsigned long long on ppc64
>(it is unsigned long)?
No, why would it?
If you do
char c = 1;
do you expect a warning? The right side of the assignent
is an "int", and the left side is a "char", but it's perfectly ok to
assign a wider type to a narrower one.
And so if "__u64" were to be a plain "unsigned long" on a 64-bit
architecture (and even if "unsigned long long" were to be 128 bits), the
constant 0xFFFFFFFFFFFFFFFFULL is (a) a perfectly valid unsigned long
long value and (b) fits perfectly well even in an "unsigned long", so
the compiler has no reason to complain about the assignment losing bits
(which it otherwise might do).
So I'd much rather make the constants too big than too small. And yes,
Andrew's suggestion about just assigning -1 works, but it's actually a
very subtle cast at that point.
Linus
Hi Andrew.
>> Although it fixes it for building on 32 bit architectures,
>> won't changing
>>
>> __u64 uid = 0xFFFFFFFFFFFFFFFF;
>>
>> to
>>
>> __u64 uid = 0xFFFFFFFFFFFFFFFFULL;
>>
>> generate a type mismatch warning on ppc64 and similar 64
>> bit architectures since __u64 is not a unsigned long long
>> on ppc64 (it is unsigned long)?
> u64 uid = -1;
>
> will work just nicely.
Won't that generate a warning about assigning a signed quantity
to an unsigned variable?
What's really needed is a set of definitions along the lines of
#define MAX_U32 ((__u32) 0xFFFFFFFFUL)
#define MAX_U64 ((__u64) 0xFFFFFFFFFFFFFFFFULL)
but as an intermediate measure, how about...
__u64 uid = ((__u64) 0xFFFFFFFFFFFFFFFFULL);
Best wishes from Riley.
---
* Nothing as pretty as a smile, nothing as ugly as a frown.
---
Outgoing mail is certified Virus Free.
Checked by AVG anti-virus system (http://www.grisoft.com).
Version: 6.0.488 / Virus Database: 287 - Release Date: 5-Jun-2003
Steve French <[email protected]> said:
> Although it fixes it for building on 32 bit architectures, won't changing
>
>
> __u64 uid = 0xFFFFFFFFFFFFFFFF;
> to
>
> __u64 uid = 0xFFFFFFFFFFFFFFFFULL;
>
> generate a type mismatch warning on ppc64 and similar 64 bit
> architecutres since __u64 is not a unsigned long long on ppc64
> (it is unsigned long)? My gut reaction is to just ingore the three
> places that cause warnings and the remaining two places that cause
> signed/unsigned compare warnings of unsigned int local variables
> to #defined literals (which presumably are treated as signed by default).
Be careful, the value will get shoehorned into 4 bytes to make the int
constant, which is then assigned to the __u64.
--
Dr. Horst H. von Brand User #22616 counter.li.org
Departamento de Informatica Fono: +56 32 654431
Universidad Tecnica Federico Santa Maria +56 32 654239
Casilla 110-V, Valparaiso, Chile Fax: +56 32 797513