net/dccp/timer.c: use 'u64' instead of 's64' to avoid compiler's warning

PATCH In review Chen Gang 3 participants 8 messages May 21, 2014 – May 23, 2014

Subsystems (2)

Networking Network drivers

lore

Share & tools

lore

Chen Gang

author

2014-05-21 00:19 UTC #0 lore raw

PATCH 1/1 net/dccp/timer.c: use 'u64' instead of 's64' to avoid compiler's warning

'dccp_timestamp_seed' is initialized once by ktime_get_real() in dccp_timestamping_init(). It is always less than ktime_get_real() in dccp_timestamp().

Then, ktime_us_delta() in dccp_timestamp() will always return positive number. So can use manual type cast to let compiler and do_div() know about it to avoid warning.

The related warning (with allmodconfig under unicore32):

CC [M] net/dccp/timer.o
net/dccp/timer.c: In function ‘dccp_timestamp’:
net/dccp/timer.c:285: warning: comparison of distinct pointer types lacks a cast

Signed-off-by Chen Gang <[email protected]>

---
 net/dccp/timer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

net/dccp/timer.c +1 -1

index 16f0b22..1cd46a3 100644
@@ -280,7 +280,7 @@ static ktime_t dccp_timestamp_seed;
  */
 u32 dccp_timestamp(void)
 {
-	s64 delta = ktime_us_delta(ktime_get_real(), dccp_timestamp_seed);
+	u64 delta = (u64)ktime_us_delta(ktime_get_real(), dccp_timestamp_seed);
 
 	do_div(delta, 10);
 	return delta;

Signature · 1 line

1.9.2.459.g68773ac

管雪涛

2014-05-22 00:26 UTC #1 lore raw

Quoted 25 lines · ----- Chen Gang

'dccp_timestamp_seed' is initialized once by ktime_get_real() in dccp_timestamping_init(). It is always less than ktime_get_real() in dccp_timestamp().
Then, ktime_us_delta() in dccp_timestamp() will always return positive number. So can use manual type cast to let compiler and do_div() know about it to avoid warning.
The related warning (with allmodconfig under unicore32):
CC [M] net/dccp/timer.o
net/dccp/timer.c: In function ‘dccp_timestamp’:
net/dccp/timer.c:285: warning: comparison of distinct pointer types lacks a cast
Signed-off-by Chen Gang <[email protected]>
---
 net/dccp/timer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
net/dccp/timer.c +0 -0
index 16f0b22..1cd46a3 100644

net/dccp/timer.c static ktime_t dccp_timestamp_seed; · line 283

Quoted 5 lines

net/dccp/timer.c +1 -1

@@ -280,7 +280,7 @@ static ktime_t dccp_timestamp_seed;
  */
 u32 dccp_timestamp(void)
 {
-	s64 delta = ktime_us_delta(ktime_get_real(), dccp_timestamp_seed);
+	u64 delta = (u64)ktime_us_delta(ktime_get_real(), dccp_timestamp_seed);

Do you assume that delta should be very small? Otherwise, return value will be different if data type is changed.

Quoted 5 lines

net/dccp/timer.c +0 -0
 	do_div(delta, 10);
 	return delta;
Signature · 1 line
1.9.2.459.g68773ac

Chen Gang

author

2014-05-22 01:01 UTC #2 lore raw

Quoted 22 lines · 管雪涛

Quoted 20 lines · ----- Chen Gang
'dccp_timestamp_seed' is initialized once by ktime_get_real() in dccp_timestamping_init(). It is always less than ktime_get_real() in dccp_timestamp().
Then, ktime_us_delta() in dccp_timestamp() will always return positive number. So can use manual type cast to let compiler and do_div() know about it to avoid warning.
The related warning (with allmodconfig under unicore32):
CC [M] net/dccp/timer.o
net/dccp/timer.c: In function ‘dccp_timestamp’:
net/dccp/timer.c:285: warning: comparison of distinct pointer types lacks a cast
Signed-off-by Chen Gang <[email protected]>
---
 net/dccp/timer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Quoted 14 lines

net/dccp/timer.c static ktime_t dccp_timestamp_seed; · line 283
Quoted 10 lines
net/dccp/timer.c +1 -1
index 16f0b22..1cd46a3 100644
@@ -280,7 +280,7 @@ static ktime_t dccp_timestamp_seed;
  */
 u32 dccp_timestamp(void)
 {
-	s64 delta = ktime_us_delta(ktime_get_real(), dccp_timestamp_seed);
+	u64 delta = (u64)ktime_us_delta(ktime_get_real(), dccp_timestamp_seed);
Do you assume that delta should be very small? Otherwise, return value will be different if data type is changed.

'u64' is a very very large number. after calculation, if it is based on nano second (although I am not quite sure whether it is based on it).

        a hour,            3,600,000,000,000ns
        a day,            90,000,000,000,000ns
        a year,       50,000,000,000,000,000ns
        10 years,    500,000,000,000,000,000ns
        100 years, 5,000,000,000,000,000,000ns
        4G * 4G = 16,000,000,000,000,000,000ns

So we can assume it will never overflow for 'u64'.

Thanks.

Signature · 3 lines

Chen Gang

Open, share, and attitude like air, water, and life which God blessed

管雪涛

2014-05-22 01:06 UTC #3 lore raw

Quoted 41 lines · ----- Chen Gang

Quoted 22 lines · 管雪涛
Quoted 20 lines · ----- Chen Gang
'dccp_timestamp_seed' is initialized once by ktime_get_real() in dccp_timestamping_init(). It is always less than ktime_get_real() in dccp_timestamp().
Then, ktime_us_delta() in dccp_timestamp() will always return positive number. So can use manual type cast to let compiler and do_div() know about it to avoid warning.
The related warning (with allmodconfig under unicore32):
CC [M] net/dccp/timer.o
net/dccp/timer.c: In function ‘dccp_timestamp’:
net/dccp/timer.c:285: warning: comparison of distinct pointer types lacks a cast
Signed-off-by Chen Gang <[email protected]>
---
 net/dccp/timer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 14 lines
net/dccp/timer.c static ktime_t dccp_timestamp_seed; · line 283
Quoted 10 lines
net/dccp/timer.c +1 -1
index 16f0b22..1cd46a3 100644
@@ -280,7 +280,7 @@ static ktime_t dccp_timestamp_seed;
  */
 u32 dccp_timestamp(void)
 {
-	s64 delta = ktime_us_delta(ktime_get_real(), dccp_timestamp_seed);
+	u64 delta = (u64)ktime_us_delta(ktime_get_real(), dccp_timestamp_seed);
Do you assume that delta should be very small? Otherwise, return value will be different if data type is changed.
'u64' is a very very large number. after calculation, if it is based on nano second (although I am not quite sure whether it is based on it).

Quoted 8 lines

        a hour,            3,600,000,000,000ns
        a day,            90,000,000,000,000ns
        a year,       50,000,000,000,000,000ns
        10 years,    500,000,000,000,000,000ns
        100 years, 5,000,000,000,000,000,000ns
        4G * 4G = 16,000,000,000,000,000,000ns

So we can assume it will never overflow for 'u64'.

However, return value of dccp_timestamp function is u32.

Quoted 7 lines

Thanks.

Signature · 3 lines

Chen Gang

Open, share, and attitude like air, water, and life which God blessed

Chen Gang

author

2014-05-22 01:14 UTC #4 lore raw

Quoted 50 lines · 管雪涛

Quoted 48 lines · ----- Chen Gang
Quoted 22 lines · 管雪涛
Quoted 20 lines · ----- Chen Gang
'dccp_timestamp_seed' is initialized once by ktime_get_real() in dccp_timestamping_init(). It is always less than ktime_get_real() in dccp_timestamp().
Then, ktime_us_delta() in dccp_timestamp() will always return positive number. So can use manual type cast to let compiler and do_div() know about it to avoid warning.
The related warning (with allmodconfig under unicore32):
CC [M] net/dccp/timer.o
net/dccp/timer.c: In function ‘dccp_timestamp’:
net/dccp/timer.c:285: warning: comparison of distinct pointer types lacks a cast
Signed-off-by Chen Gang <[email protected]>
---
 net/dccp/timer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 14 lines
net/dccp/timer.c static ktime_t dccp_timestamp_seed; · line 283
Quoted 10 lines
net/dccp/timer.c +1 -1
index 16f0b22..1cd46a3 100644
@@ -280,7 +280,7 @@ static ktime_t dccp_timestamp_seed;
  */
 u32 dccp_timestamp(void)
 {
-	s64 delta = ktime_us_delta(ktime_get_real(), dccp_timestamp_seed);
+	u64 delta = (u64)ktime_us_delta(ktime_get_real(), dccp_timestamp_seed);
Do you assume that delta should be very small? Otherwise, return value will be different if data type is changed.
'u64' is a very very large number. after calculation, if it is based on nano second (although I am not quite sure whether it is based on it).
        a hour,            3,600,000,000,000ns
        a day,            90,000,000,000,000ns
        a year,       50,000,000,000,000,000ns
        10 years,    500,000,000,000,000,000ns
        100 years, 5,000,000,000,000,000,000ns
        4G * 4G = 16,000,000,000,000,000,000ns

Quoted 4 lines

Quoted 1 line
So we can assume it will never overflow for 'u64'.
However, return value of dccp_timestamp function is u32.

After check the function comments.

/**
* dccp_timestamp - 10s of microseconds time source
* Returns the number of 10s of microseconds since loading DCCP. This is native
* DCCP time difference format (RFC 4340, sec. 13).
* Please note: This will wrap around about circa every 11.9 hours. */

So, it is still acceptable, although it is truncated into 'u32' before return.

Quoted 7 lines

Quoted 7 lines
Thanks.
Signature · 3 lines
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

Signature · 3 lines

Chen Gang

Open, share, and attitude like air, water, and life which God blessed

David Miller

maintainer

2014-05-22 19:33 UTC #5 lore raw

In reply to Chen Gang

RnJvbTogQ2hlbiBHYW5nIDxnYW5nLmNoZW4uNWk1akBnbWFpbC5jb20+DQpEYXRlOiBXZWQsIDIx IE1heSAyMDE0IDA4OjE5OjM0ICswODAwDQoNCj4gJ2RjY3BfdGltZXN0YW1wX3NlZWQnIGlzIGlu aXRpYWxpemVkIG9uY2UgYnkga3RpbWVfZ2V0X3JlYWwoKSBpbg0KPiBkY2NwX3RpbWVzdGFtcGlu Z19pbml0KCkuIEl0IGlzIGFsd2F5cyBsZXNzIHRoYW4ga3RpbWVfZ2V0X3JlYWwoKQ0KPiBpbiBk Y2NwX3RpbWVzdGFtcCgpLg0KPiANCj4gVGhlbiwga3RpbWVfdXNfZGVsdGEoKSBpbiBkY2NwX3Rp bWVzdGFtcCgpIHdpbGwgYWx3YXlzIHJldHVybiBwb3NpdGl2ZQ0KPiBudW1iZXIuIFNvIGNhbiB1 c2UgbWFudWFsIHR5cGUgY2FzdCB0byBsZXQgY29tcGlsZXIgYW5kIGRvX2RpdigpIGtub3cNCj4g YWJvdXQgaXQgdG8gYXZvaWQgd2FybmluZy4NCj4gDQo+IFRoZSByZWxhdGVkIHdhcm5pbmcgKHdp dGggYWxsbW9kY29uZmlnIHVuZGVyIHVuaWNvcmUzMik6DQo+IA0KPiAgICAgQ0MgW01dICBuZXQv ZGNjcC90aW1lci5vDQo+ICAgbmV0L2RjY3AvdGltZXIuYzogSW4gZnVuY3Rpb24goWRjY3BfdGlt ZXN0YW1wojoNCj4gICBuZXQvZGNjcC90aW1lci5jOjI4NTogd2FybmluZzogY29tcGFyaXNvbiBv ZiBkaXN0aW5jdCBwb2ludGVyIHR5cGVzIGxhY2tzIGEgY2FzdA0KPiANCj4gDQo+IFNpZ25lZC1v ZmYtYnk6IENoZW4gR2FuZyA8Z2FuZy5jaGVuLjVpNWpAZ21haWwuY29tPg0KDQpBcHBsaWVkIHRv IG5ldC1uZXh0LCB0aGFua3MuDQoNCkJ1dCB0aGF0IHR5cGUgY2hlY2sgaW4gaW5jbHVkZS9hc20t Z2VuZXJpYy9kaXY2NC5oIGlzIGJvZ3VzLCBpdCBzaG91bGQNCmJlIGNoZWNraW5nIHNpemVvZihY KSA9PSA4IHJhdGhlciB0aGFuIHRoZSB0eXBlIHRoaW5nLCBpdCBqdXN0IHdhbnRzIHRvDQptYWtl IHN1cmUgdGhhdCB0aGUgdmFsdWUgaXMgNjQtYml0IHJlZ2FyZGxlc3Mgb2YgaXQncyBzaWduZWRu ZXNzLg0KDQpUaGUgYXJjaCBsb2NhbCBpbXBsZW1lbnRhdGlvbnMgZG8gbm90IGRvIHRoaXMsIGFu ZCB0aGF0J3Mgd2h5IHZlcnkgZmV3DQpvdGhlciBwZW9wbGUgbm90aWNlIHRoaXMgd2FybmluZy4N Cg=

管雪涛

2014-05-22 23:58 UTC #6 lore raw

Quoted 20 lines · ----- David Miller

From: Chen Gang <[email protected]>
Date: Wed, 21 May 2014 08:19:34 +0800
Quoted 8 lines
'dccp_timestamp_seed' is initialized once by ktime_get_real() in dccp_timestamping_init(). It is always less than ktime_get_real() in dccp_timestamp().
Then, ktime_us_delta() in dccp_timestamp() will always return positive number. So can use manual type cast to let compiler and do_div() know about it to avoid warning.
Quoted 8 lines
The related warning (with allmodconfig under unicore32):
CC [M] net/dccp/timer.o
net/dccp/timer.c: In function ‘dccp_timestamp’:
net/dccp/timer.c:285: warning: comparison of distinct pointer types lacks a cast
Signed-off-by Chen Gang <[email protected]>

Quoted 8 lines

Applied to net-next, thanks.
But that type check in include/asm-generic/div64.h is bogus, it should be checking sizeof(X) = 8 rather than the type thing, it just wants to make sure that the value is 64-bit regardless of it's signedness.
The arch local implementations do not do this, and that's why very few other people notice this warning.

Arch-dependent codes implement it with unsigned long long type. And, every warning should not be ignored.

Chen Gang

author

2014-05-23 01:43 UTC #7 lore raw

Quoted 20 lines · 管雪涛

Quoted 18 lines · ----- David Miller
From: Chen Gang <[email protected]>
Date: Wed, 21 May 2014 08:19:34 +0800
Quoted 15 lines
'dccp_timestamp_seed' is initialized once by ktime_get_real() in dccp_timestamping_init(). It is always less than ktime_get_real() in dccp_timestamp().
Then, ktime_us_delta() in dccp_timestamp() will always return positive number. So can use manual type cast to let compiler and do_div() know about it to avoid warning.
The related warning (with allmodconfig under unicore32):
CC [M] net/dccp/timer.o
net/dccp/timer.c: In function ‘dccp_timestamp’:
net/dccp/timer.c:285: warning: comparison of distinct pointer types lacks a cast

Quoted 4 lines

Quoted 4 lines
Quoted 1 line
Signed-off-by Chen Gang <[email protected]>
Applied to net-next, thanks.

Thank you for your work.

Quoted 10 lines

Quoted 6 lines
But that type check in include/asm-generic/div64.h is bogus, it should be checking sizeof(X) = 8 rather than the type thing, it just wants to make sure that the value is 64-bit regardless of it's signedness.
The arch local implementations do not do this, and that's why very few other people notice this warning.
Arch-dependent codes implement it with unsigned long long type. And, every warning should not be ignored.

Yeah, we have to let do_div() no touch (especially for 32-bit machine, which the highest bit is checked). The related code in "include/asm-generic/div64.h":

23 #if BITS_PER_LONG = 64
24

 25 # define do_div(n,base) ({                                      \
 26         uint32_t __base = (base);                               \
 27         uint32_t __rem;                                         \
 28         __rem = ((uint64_t)(n)) % __base;                       \
 29         (n) = ((uint64_t)(n)) / __base;                         \
 30         __rem;                                                  \
 31  })
 32
 33 #elif BITS_PER_LONG = 32
 34
 35 extern uint32_t __div64_32(uint64_t *dividend, uint32_t divisor);
 36
 37 /* The unnecessary pointer compare is there
 38  * to check for type safety (n must be 64bit)
 39  */
 40 # define do_div(n,base) ({                              \
 41         uint32_t __base = (base);                       \
 42         uint32_t __rem;                                 \
 43         (void)(((typeof((n)) *)0) = ((uint64_t *)0));  \
 44         if (likely(((n) >> 32) = 0)) {                 \
 45                 __rem = (uint32_t)(n) % __base;         \
 46                 (n) = (uint32_t)(n) / __base;           \
 47         } else                                          \
 48                 __rem = __div64_32(&(n), __base);       \
 49         __rem;                                          \

50 })

And for division operation, architectures are signed/unsigned sensitive, e.g. div_u64() and div_s64(), they are different.

Thanks.

Signature · 3 lines

Chen Gang

Open, share, and attitude like air, water, and life which God blessed