login
Header Space

 
 

Re: [PATCH] kernel/time.c: Silence gcc warning 'integer constant to large for long type'

Previous thread: Resend [Patch 1/1] Trivial - kernel/time/ntp.c: fix warning by debian developer on Friday, May 2, 2008 - 2:40 pm. (1 message)

Next thread: [PATCH 01/12] lib: add ascii hex helper functions by Harvey Harrison on Friday, May 2, 2008 - 3:01 pm. (1 message)
To: Linus Torvalds <torvalds@...>
Cc: Carlos R. Mafra <crmafra@...>, Linux Kernel Mailing List <linux-kernel@...>
Date: Friday, May 2, 2008 - 2:58 pm

From: Carlos R. Mafra &lt;crmafra@ift.unesp.br&gt;

This patch removes these gcc warnings:

kernel/time.c: In function msecs_to_jiffies:
kernel/time.c:479: warning: integer constant is too large for long type
kernel/time.c: In function usecs_to_jiffies:
kernel/time.c:494: warning: integer constant is too large for long type

by casting MSEC_TO_HZ_ADJ32 and USEC_TO_HZ_ADJ32 to u64.

This resolves bugzilla 10153.

Signed-off-by: Carlos R. Mafra &lt;crmafra@ift.unesp.br&gt;
Signed-off-by: H. Peter Anvin &lt;hpa@zytor.com&gt;
---
 kernel/time.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/time.c b/kernel/time.c
index cbe0d5a..76dcc0f 100644
--- a/kernel/time.c
+++ b/kernel/time.c
@@ -476,7 +476,7 @@ unsigned long msecs_to_jiffies(const unsigned int m)
 	if (HZ &gt; MSEC_PER_SEC &amp;&amp; m &gt; jiffies_to_msecs(MAX_JIFFY_OFFSET))
 		return MAX_JIFFY_OFFSET;
 
-	return ((u64)MSEC_TO_HZ_MUL32 * m + MSEC_TO_HZ_ADJ32)
+	return ((u64)MSEC_TO_HZ_MUL32 * m + (u64)MSEC_TO_HZ_ADJ32)
 		&gt;&gt; MSEC_TO_HZ_SHR32;
 #endif
 }
@@ -491,7 +491,7 @@ unsigned long usecs_to_jiffies(const unsigned int u)
 #elif HZ &gt; USEC_PER_SEC &amp;&amp; !(HZ % USEC_PER_SEC)
 	return u * (HZ / USEC_PER_SEC);
 #else
-	return ((u64)USEC_TO_HZ_MUL32 * u + USEC_TO_HZ_ADJ32)
+	return ((u64)USEC_TO_HZ_MUL32 * u + (u64)USEC_TO_HZ_ADJ32)
 		&gt;&gt; USEC_TO_HZ_SHR32;
 #endif
 }
-- 
1.5.4.3



--
To: H. Peter Anvin <hpa@...>
Cc: Carlos R. Mafra <crmafra@...>, Linux Kernel Mailing List <linux-kernel@...>
Date: Friday, May 2, 2008 - 5:14 pm

Shouldn't we fix the perl-script to mark the constants appropriately 
typed? Ie add the proper "ul" or "ull" endings there as necessary?

For example, I see

	#define MSEC_TO_HZ_ADJ64        0x18000000000000000

in the auto-generated timeconst.h file, and the fact is, that's a really 
really ugly constant. It simply doesn't even fit in a u64. Why do these 
kinds of pointless and useless #define even get generated, when using them 
would inevitably be a bug anyway?

As to the ones that *do* fit in 64 bits, they should still haev the 
correct "ul" and "ull" endings on 64- and 32-bit architectures 
respectively. Yeah, hex constants are always unsigned and the compiler 
will expand them to the right size, but the compiler is also rigth to warn 
about it (and casting them shouldn't even change that fact, even if it 
happens to do so with gcc).

		Linus
--
To: Linus Torvalds <torvalds@...>
Cc: Carlos R. Mafra <crmafra@...>, Linux Kernel Mailing List <linux-kernel@...>
Date: Friday, May 2, 2008 - 5:33 pm

That's more or less what my other patchset does.  It's a bit more 
complex (because it gets the suffixes via macros, to get the right 
suffixes), but most of it is pure cleanup of the way we handle integers 
in architecture includes:

git://git.kernel.org/pub/scm/linux/kernel/git/hpa/linux-2.6-inttypes.git

	-hpa
--
To: H. Peter Anvin <hpa@...>
Cc: Carlos R. Mafra <crmafra@...>, Linux Kernel Mailing List <linux-kernel@...>
Date: Friday, May 2, 2008 - 6:17 pm

This is *still* wrong.

That C type prefix should be at the *define*, not the use of those macros.

The thing is, if C code needs to do

	U64_C(HZ_TO_USEC_MUL32)

to use the macro HZ_TO_USEC_MUL32, then that is a *bug* in the macro.

That kind of stuff should not be visible to users of plain constants!

		Linus
--
To: H. Peter Anvin <hpa@...>
Cc: Carlos R. Mafra <crmafra@...>, Linux Kernel Mailing List <linux-kernel@...>
Date: Friday, May 2, 2008 - 6:33 pm

Ok, it seems worse than that. I don't see the point of that U64_C thing at 
all.

Those macros are only used in C code. 

The values should have the right C types already (ie "ull" at the end of 
big constants to make sure we don't trigger warnings). And no, we do NOT 
want to have 5 different macro names for five different versions of the 
same macro. That's just insane.

Make the timeconst.h file just contain sane macros. No preprocessor games 
etc. Just make it say

	#define USEC_TO_HZ_MUL32 ..correct-value-here..

and not even generate macros with values that cannot be used (ie &gt;64 
bits).

		Linus
--
To: Linus Torvalds <torvalds@...>
Cc: Carlos R. Mafra <crmafra@...>, Linux Kernel Mailing List <linux-kernel@...>
Date: Friday, May 2, 2008 - 7:23 pm

Updated git tree now available.  The timeconst patch (the only one that 
has changed) is attached.

git://git.kernel.org/pub/scm/linux/kernel/git/hpa/linux-2.6-inttypes.git

	-hpa
To: H. Peter Anvin <hpa@...>
Cc: Carlos R. Mafra <crmafra@...>, Linux Kernel Mailing List <linux-kernel@...>
Date: Saturday, May 3, 2008 - 1:57 pm

Thanks, much better. Pulled.

		Linus
--
To: Linus Torvalds <torvalds@...>
Cc: Carlos R. Mafra <crmafra@...>, Linux Kernel Mailing List <linux-kernel@...>
Date: Friday, May 2, 2008 - 6:45 pm

OK.  I was being too general, I guess (holding out for stuff like doing 
this in 64-bit space which would require 128-bit constants, which gcc 
*does* support on 64-bit platforms.)

I'll have a clean patch shortly.

	-hpa
--
Previous thread: Resend [Patch 1/1] Trivial - kernel/time/ntp.c: fix warning by debian developer on Friday, May 2, 2008 - 2:40 pm. (1 message)

Next thread: [PATCH 01/12] lib: add ascii hex helper functions by Harvey Harrison on Friday, May 2, 2008 - 3:01 pm. (1 message)
speck-geostationary