use 64-bit arithmetic instead of 32-bit
Subsystems (8)
v3 cover use 64-bit arithmetic instead of 32-bit
Add suffix LL and ULL to various constants in order to give the compiler complete information about the proper arithmetic to use. Such constants are used in contexts that expect expressions of type u64 (64 bits, unsigned) and s64 (64 bits, signed).
The mentioned expressions are currently being evaluated using 32-bit arithmetic, wich in some cases can lead to unintentional integer overflows.
This patchset addresses the following "Unintentional integer overflow" Coverity reports:
CIDs: 200604, 1056807, 1056808, 1271223, 1324146
CIDs: 1392628, 1392630, 1446589, 1454996, 1458347
Thank you
Changes in v2:
- Update subject and changelog to better reflect the proposed code changes.
- Add suffix ULL and LL to constants instead of casting variables.
- Extend the proposed code changes to other similar cases that had not previously been considered in v1 of this patchset.
Changes in v3:
- Mention the specific Coverity report in all commit messages.
Gustavo A. R. Silva (8):
rtl2832: use 64-bit arithmetic instead of 32-bit in
rtl2832_set_frontend
dvb-frontends: ves1820: use 64-bit arithmetic instead of 32-bit
i2c: max2175: use 64-bit arithmetic instead of 32-bit
i2c: ov9650: use 64-bit arithmetic instead of 32-bit
pci: cx88-input: use 64-bit arithmetic instead of 32-bit rockchip/rga: use 64-bit arithmetic instead of 32-bit
platform: sh_veu: use 64-bit arithmetic instead of 32-bit
platform: vivid-cec: use 64-bit arithmetic instead of 32-bit
drivers/media/dvb-frontends/rtl2832.c | 4 ++-- drivers/media/dvb-frontends/ves1820.c | 2 +- drivers/media/i2c/max2175.c | 2 +- drivers/media/i2c/ov9650.c | 9 +++++---- drivers/media/pci/cx88/cx88-input.c | 4 ++-- drivers/media/platform/rockchip/rga/rga-buf.c | 3 ++- drivers/media/platform/sh_veu.c | 4 ++-- drivers/media/platform/vivid/vivid-cec.c | 11 +++++++++-- 8 files changed, 24 insertions(+), 15 deletions(-)
Signature · 1 line
2.7.4
PATCH v3 1/8 rtl2832: use 64-bit arithmetic instead of 32-bit in rtl2832_set_frontend
Add suffix ULL to constant 7 in order to give the compiler complete information about the proper arithmetic to use. Notice that this constant is used in a context that expects an expression of type u64 (64 bits, unsigned).
The expression dev->pdata->clk * 7 is currently being evaluated using 32-bit arithmetic.
Addresses-Coverity-ID: 1271223 ("Unintentional integer overflow")
---
Changes in v2:
- Update subject and changelog to better reflect the proposed code changes.
- Add suffix ULL to constant instead of casting a variable.
Changes in v3:
- Mention the specific Coverity report in the commit message.
drivers/media/dvb-frontends/rtl2832.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/media/dvb-frontends/rtl2832.c +2 -2
@@ -498,7 +498,7 @@ static int rtl2832_set_frontend(struct dvb_frontend *fe)* RSAMP_RATIO = floor(CrystalFreqHz * 7 * pow(2, 22) * / ConstWithBandwidthMode) */ - num = dev->pdata->clk *7 ; + num = dev->pdata->clk *7ULL ; num *=0x400000 ; num =div_u64 (num, bw_mode); resamp_ratio = num &0x3ffffff ;
@@ -511,7 +511,7 @@ static int rtl2832_set_frontend(struct dvb_frontend *fe)* / (CrystalFreqHz * 7)) */ num = bw_mode <<20 ; - num2 = dev->pdata->clk *7 ; + num2 = dev->pdata->clk *7ULL ; num =div_u64 (num, num2); num = -num; cfreq_off_ratio = num &0xfffff ;
Signature · 1 line
2.7.4
PATCH v3 2/8 dvb-frontends: ves1820: use 64-bit arithmetic instead of 32-bit
In reply to Gustavo A. R. SilvaAdd suffix ULL to constant 10 in order to give the compiler complete information about the proper arithmetic to use. Notice that this constant is used in a context that expects an expression of type u64 (64 bits, unsigned).
The expression fpxin = state->config->xin * 10 is currently being evaluated using 32-bit arithmetic.
Addresses-Coverity-ID: 200604 ("Unintentional integer overflow")
---
Changes in v2:
- Update subject and changelog to better reflect the proposed code changes.
- Add suffix ULL to constant instead of casting a variable.
Changes in v3:
- Mention the specific Coverity report in the commit message.
drivers/media/dvb-frontends/ves1820.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/media/dvb-frontends/ves1820.c +1 -1
@@ -137,7 +137,7 @@ static int ves1820_set_symbolrate(struct ves1820_state *state, u32 symbolrate)NDEC =3 ;/* yeuch! */ - fpxin = state->config->xin *10 ; + fpxin = state->config->xin *10ULL ; fptmp = fpxin;do_div (fptmp,123 );if (symbolrate < fptmp)SFIL =1 ;
Signature · 1 line
2.7.4
PATCH v3 3/8 i2c: max2175: use 64-bit arithmetic instead of 32-bit
In reply to Gustavo A. R. SilvaAdd suffix LL to constant 2 in order to give the compiler complete information about the proper arithmetic to use. Notice that this constant is used in a context that expects an expression of type s64 (64 bits, signed).
The expression 2 * (clock_rate - abs_nco_freq) is currently being evaluated using 32-bit arithmetic.
Addresses-Coverity-ID: 1446589 ("Unintentional integer overflow")
---
Changes in v2:
- Update subject and changelog to better reflect the proposed code changes.
- Add suffix LL to constant instead of casting a variable.
Changes in v3:
- Mention the specific Coverity report in the commit message.
drivers/media/i2c/max2175.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/media/i2c/max2175.c +1 -1
@@ -643,7 +643,7 @@ static int max2175_set_nco_freq(struct max2175 *ctx, s32 nco_freq)if (abs_nco_freq < clock_rate /2 ) { nco_val_desired =2 * nco_freq; }else { - nco_val_desired =2 * (clock_rate - abs_nco_freq); + nco_val_desired =2LL * (clock_rate - abs_nco_freq);if (nco_freq <0 ) nco_val_desired = -nco_val_desired; }
Signature · 1 line
2.7.4
PATCH v3 4/8 i2c: ov9650: use 64-bit arithmetic instead of 32-bit
In reply to Gustavo A. R. SilvaAdd suffix ULL to constants 10000 and 1000000 in order to give the compiler complete information about the proper arithmetic to use. Notice that these constants are used in contexts that expect expressions of type u64 (64 bits, unsigned).
The following expressions:
(u64)(fi->interval.numerator * 10000) (u64)(iv->interval.numerator * 10000) fiv->interval.numerator * 1000000 / fiv->interval.denominator
are currently being evaluated using 32-bit arithmetic.
Notice that those casts to u64 for the first two expressions are only effective after such expressions are evaluated using 32-bit arithmetic, which leads to potential integer overflows. So based on those casts, it seems that the original intention of the code is to actually use 64-bit arithmetic instead of 32-bit.
Also, notice that once the suffix ULL is added to the constants, the outer casts to u64 are no longer needed.
Addresses-Coverity-ID: 1324146 ("Unintentional integer overflow")
---
Changes in v2:
- Update subject and changelog to better reflect the proposed code changes.
- Add suffix ULL to constants instead of casting variables.
- Remove unnecessary casts to u64 as part of the code change.
- Extend the same code change to other similar expressions.
Changes in v3:
- None.
drivers/media/i2c/ov9650.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
drivers/media/i2c/ov9650.c +5 -4
@@ -1130,7 +1130,7 @@ static int __ov965x_set_frame_interval(struct ov965x *ov965x,if (fi->interval.denominator ==0 )return -EINVAL ; - req_int = (u64)(fi->interval.numerator *10000 ) / + req_int = fi->interval.numerator *10000ULL / fi->interval.denominator;for (i =0 ; i <ARRAY_SIZE (ov965x_intervals); i++) {
@@ -1139,7 +1139,7 @@ static int __ov965x_set_frame_interval(struct ov965x *ov965x,if (mbus_fmt->width != iv->size.width || mbus_fmt->height != iv->size.height)continue ; - err =abs ((u64)(iv->interval.numerator *10000 ) / + err =abs (iv->interval.numerator *10000ULL / iv->interval.denominator - req_int);if (err < min_err) { fiv = iv;
@@ -1148,8 +1148,9 @@ static int __ov965x_set_frame_interval(struct ov965x *ov965x, } ov965x->fiv = fiv; -v4l2_dbg (1 , debug, &ov965x->sd,"Changed frame interval to %u us\n" , - fiv->interval.numerator *1000000 / fiv->interval.denominator); +v4l2_dbg (1 , debug, &ov965x->sd,"Changed frame interval to %llu us\n" , + fiv->interval.numerator *1000000ULL / + fiv->interval.denominator);return 0 ; }
Signature · 1 line
2.7.4
Re: [PATCH v3 4/8] i2c: ov9650: use 64-bit arithmetic instead of 32-bit
Hi Gustavo,
Quoted 44 lines · Gustavo A. R. Silva
Add suffix ULL to constants 10000 and 1000000 in order to give the compiler complete information about the proper arithmetic to use. Notice that these constants are used in contexts that expect expressions of type u64 (64 bits, unsigned).
The following expressions:
(u64)(fi->interval.numerator * 10000) (u64)(iv->interval.numerator * 10000)
fiv->interval.numerator* 1000000 /fiv->interval.denominatorare currently being evaluated using 32-bit arithmetic.
Notice that those casts to u64 for the first two expressions are only effective after such expressions are evaluated using 32-bit arithmetic, which leads to potential integer overflows. So based on those casts, it seems that the original intention of the code is to actually use 64-bit arithmetic instead of 32-bit.
Also, notice that once the suffix ULL is added to the constants, the outer casts to u64 are no longer needed.
Addresses-Coverity-ID: 1324146 ("Unintentional integer overflow")
Fixes 84a15ded76ec ("[media] V4L: Add driver for OV9650/52 image sensors")Fixes 79211c8ed19c ("remove abs64()")Signed-off-by Gustavo A. R. Silva <[email protected]>---
Changes in v2:
- Update subject and changelog to better reflect the proposed code changes.
- Add suffix ULL to constants instead of casting variables.
- Remove unnecessary casts to u64 as part of the code change.
- Extend the same code change to other similar expressions.Changes in v3:
- None.drivers/media/i2c/ov9650.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)drivers/media/i2c/ov9650.c +0
-0
drivers/media/i2c/ov9650.c static int __ov965x_set_frame_interval(struct ov965x *ov965x, · line 1134
Quoted 6 lines
drivers/media/i2c/ov9650.c +1
-1@@ -1130,7 +1130,7 @@ static int __ov965x_set_frame_interval(struct ov965x *ov965x,if (fi->interval.denominator ==0 )return -EINVAL ; - req_int = (u64)(fi->interval.numerator *10000 ) / + req_int = fi->interval.numerator *10000ULL / fi->interval.denominator;
This has been addressed by your earlier patch "i2c: ov9650: fix potential integer overflow in __ov965x_set_frame_interval" I tweaked a little. It's not in media tree
master yet.
drivers/media/i2c/ov9650.c static int __ov965x_set_frame_interval(struct ov965x *ov965x, · line 1142
Quoted 8 lines
drivers/media/i2c/ov9650.c +1
-1for (i =0 ; i <ARRAY_SIZE (ov965x_intervals); i++) {@@ -1139,7 +1139,7 @@ static int __ov965x_set_frame_interval(struct ov965x *ov965x,if (mbus_fmt->width != iv->size.width || mbus_fmt->height != iv->size.height)continue ; - err = abs((u64)(iv->interval.numerator * 10000) / + err = abs(iv->interval.numerator * 10000ULL /
This and the chunk below won't work on e.g. 32-bit ARM. do_div(), please.
Quoted 15 lines
drivers/media/i2c/ov9650.c +3
-2iv->interval.denominator - req_int);if (err < min_err) { fiv = iv;@@ -1148,8 +1148,9 @@ static int __ov965x_set_frame_interval(struct ov965x *ov965x, } ov965x->fiv = fiv; -v4l2_dbg (1 , debug, &ov965x->sd,"Changed frame interval to %u us\n" , - fiv->interval.numerator *1000000 / fiv->interval.denominator); +v4l2_dbg (1 , debug, &ov965x->sd,"Changed frame interval to %llu us\n" , + fiv->interval.numerator *1000000ULL / + fiv->interval.denominator);return 0 ; }
Signature · 4 lines
Regards, Sakari Ailus e-mail: [email protected]
Re: [PATCH v3 4/8] i2c: ov9650: use 64-bit arithmetic instead of 32-bit
Hi Sakari,
Quoted 50 lines · Sakari Ailus
Hi Gustavo,
Quoted 47 lines · Gustavo A. R. Silva
Add suffix ULL to constants 10000 and 1000000 in order to give the compiler complete information about the proper arithmetic to use. Notice that these constants are used in contexts that expect expressions of type u64 (64 bits, unsigned).
The following expressions:
(u64)(fi->interval.numerator * 10000) (u64)(iv->interval.numerator * 10000)
fiv->interval.numerator* 1000000 /fiv->interval.denominatorare currently being evaluated using 32-bit arithmetic.
Notice that those casts to u64 for the first two expressions are only effective after such expressions are evaluated using 32-bit arithmetic, which leads to potential integer overflows. So based on those casts, it seems that the original intention of the code is to actually use 64-bit arithmetic instead of 32-bit.
Also, notice that once the suffix ULL is added to the constants, the outer casts to u64 are no longer needed.
Addresses-Coverity-ID: 1324146 ("Unintentional integer overflow")
Fixes 84a15ded76ec ("[media] V4L: Add driver for OV9650/52 image sensors")Fixes 79211c8ed19c ("remove abs64()")Signed-off-by Gustavo A. R. Silva <[email protected]>---
Changes in v2:
- Update subject and changelog to better reflect the proposed code changes.
- Add suffix ULL to constants instead of casting variables.
- Remove unnecessary casts to u64 as part of the code change.
- Extend the same code change to other similar expressions.Changes in v3:
- None.drivers/media/i2c/ov9650.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)drivers/media/i2c/ov9650.c +0
-0@@ -1130,7 +1130,7 @@ static int __ov965x_set_frame_interval(struct ov965x *ov965x,if (fi->interval.denominator ==0 )return -EINVAL ;
Quoted 8 lines
drivers/media/i2c/ov9650.c static int __ov965x_set_frame_interval(struct ov965x *ov965x, · line 1134
Quoted 3 lines
drivers/media/i2c/ov9650.c +1
-1- req_int = (u64)(fi->interval.numerator *10000 ) / + req_int = fi->interval.numerator *10000ULL / fi->interval.denominator;This has been addressed by your earlier patch "i2c: ov9650: fix potential integer overflow in
__ov965x_set_frame_interval" I tweaked a little. It's not in media tree
master yet.
Yeah. Actually this patch is supposed to be an improved version of the one you mention. That is why this is version 3.
Also, I wonder if the same issue you mention below regarding 32-bit ARM applies in this case too?
Quoted 11 lines
drivers/media/i2c/ov9650.c static int __ov965x_set_frame_interval(struct ov965x *ov965x, · line 1142
Quoted 8 lines
drivers/media/i2c/ov9650.c +1
-1for (i =0 ; i <ARRAY_SIZE (ov965x_intervals); i++) {@@ -1139,7 +1139,7 @@ static int __ov965x_set_frame_interval(struct ov965x *ov965x,if (mbus_fmt->width != iv->size.width || mbus_fmt->height != iv->size.height)continue ; - err = abs((u64)(iv->interval.numerator * 10000) / + err = abs(iv->interval.numerator * 10000ULL /This and the chunk below won't work on e.g. 32-bit ARM.
do_div(), please.
Thanks for pointing this out.
Quoted 7 lines
Quoted 7 lines
drivers/media/i2c/ov9650.c +0
-0iv->interval.denominator - req_int);if (err < min_err) { fiv = iv;@@ -1148,8 +1148,9 @@ static int __ov965x_set_frame_interval(struct ov965x *ov965x, } ov965x->fiv = fiv;
Quoted 5 lines
drivers/media/i2c/ov9650.c static int __ov965x_set_frame_interval(struct ov965x *ov965x, · line 1153
Quoted 5 lines
drivers/media/i2c/ov9650.c +3
-2-v4l2_dbg (1 , debug, &ov965x->sd,"Changed frame interval to %u us\n" , - fiv->interval.numerator *1000000 / fiv->interval.denominator); +v4l2_dbg (1 , debug, &ov965x->sd,"Changed frame interval to %llu us\n" , + fiv->interval.numerator *1000000ULL / + fiv->interval.denominator);
I wonder if do_div should be used for the code above?
I appreciate your feedback.
Thank you!
Signature · 1 line
Gustavo
Re: [PATCH v3 4/8] i2c: ov9650: use 64-bit arithmetic instead of 32-bit
Quoted 78 lines · Gustavo A. R. Silva
Hi Sakari,
Quoted 42 lines · Sakari Ailus
Hi Gustavo,
Quoted 39 lines · Gustavo A. R. Silva
Add suffix ULL to constants 10000 and 1000000 in order to give the compiler complete information about the proper arithmetic to use. Notice that these constants are used in contexts that expect expressions of type u64 (64 bits, unsigned).
The following expressions:
(u64)(fi->interval.numerator * 10000) (u64)(iv->interval.numerator * 10000)
fiv->interval.numerator* 1000000 /fiv->interval.denominatorare currently being evaluated using 32-bit arithmetic.
Notice that those casts to u64 for the first two expressions are only effective after such expressions are evaluated using 32-bit arithmetic, which leads to potential integer overflows. So based on those casts, it seems that the original intention of the code is to actually use 64-bit arithmetic instead of 32-bit.
Also, notice that once the suffix ULL is added to the constants, the outer casts to u64 are no longer needed.
Addresses-Coverity-ID: 1324146 ("Unintentional integer overflow")
Fixes 84a15ded76ec ("[media] V4L: Add driver for OV9650/52 image sensors")Fixes 79211c8ed19c ("remove abs64()")Signed-off-by Gustavo A. R. Silva <[email protected]>---
Changes in v2:
- Update subject and changelog to better reflect the proposed code changes.
- Add suffix ULL to constants instead of casting variables.
- Remove unnecessary casts to u64 as part of the code change.
- Extend the same code change to other similar expressions.Changes in v3:
- None.
drivers/media/i2c/ov9650.c| 9 +++++----1 file changed, 5 insertions(+), 4 deletions(-)Quoted 15 lines
Quoted 10 lines
drivers/media/i2c/ov9650.c +0
-0@@ -1130,7 +1130,7 @@ static int __ov965x_set_frame_interval(struct ov965x *ov965x, if (fi->interval.denominator == 0) return -EINVAL;-
req_int= (u64)(fi->interval.numerator * 10000) / +req_int=fi->interval.numerator* 10000ULL /fi->interval.denominator;This has been addressed by your earlier patch "i2c: ov9650: fix potential integer overflow in
__ov965x_set_frame_interval" I tweaked a little. It's not in media tree
master yet.Yeah. Actually this patch is supposed to be an improved version of the one you mention. That is why this is version 3.
Also, I wonder if the same issue you mention below regarding 32-bit ARM applies in this case too?
Quoted 10 lines
Quoted 7 lines
for (i = 0; i <
ARRAY_SIZE(ov965x_intervals); i++) {drivers/media/i2c/ov9650.c +0
-0@@ -1139,7 +1139,7 @@ static int __ov965x_set_frame_interval(struct ov965x *ov965x,if (
mbus_fmt->width!=iv->size.width||mbus_fmt->height!=iv->size.height) continue;
- err = abs((u64)(iv->interval.numerator * 10000) / + err = abs(iv->interval.numerator * 10000ULL /This and the chunk below won't work on e.g. 32-bit ARM.
do_div(), please.Thanks for pointing this out.
Quoted 13 lines
Quoted 11 lines
drivers/media/i2c/ov9650.c static int __ov965x_set_frame_interval(struct ov965x *ov965x, · line 1153
Quoted 11 lines
iv->interval.denominator-req_int); if (err <min_err) {
fiv = iv;drivers/media/i2c/ov9650.c +3
-1@@ -1148,8 +1148,9 @@ static int __ov965x_set_frame_interval(struct ov965x *ov965x, } ov965x->fiv = fiv; - v4l2_dbg(1, debug, &ov965x->sd, "Changed frame interval to %u us\n", - fiv->interval.numerator * 1000000 / fiv->interval.denominator); + v4l2_dbg(1, debug, &ov965x->sd, "Changed frame interval to %llu us\n", + fiv->interval.numerator * 1000000ULL / + fiv->interval.denominator);I wonder if
do_divshould be used for the code above?
Yes, do_div should be used.
Hans
Quoted 4 lines
I appreciate your feedback.
Thank you!
Re: [PATCH v3 4/8] i2c: ov9650: use 64-bit arithmetic instead of 32-bit
Quoted 93 lines · Hans Verkuil
Quoted 78 lines · Gustavo A. R. Silva
Hi Sakari,
Quoted 42 lines · Sakari Ailus
Hi Gustavo,
Quoted 39 lines · Gustavo A. R. Silva
Add suffix ULL to constants 10000 and 1000000 in order to give the compiler complete information about the proper arithmetic to use. Notice that these constants are used in contexts that expect expressions of type u64 (64 bits, unsigned).
The following expressions:
(u64)(fi->interval.numerator * 10000) (u64)(iv->interval.numerator * 10000)
fiv->interval.numerator* 1000000 /fiv->interval.denominatorare currently being evaluated using 32-bit arithmetic.
Notice that those casts to u64 for the first two expressions are only effective after such expressions are evaluated using 32-bit arithmetic, which leads to potential integer overflows. So based on those casts, it seems that the original intention of the code is to actually use 64-bit arithmetic instead of 32-bit.
Also, notice that once the suffix ULL is added to the constants, the outer casts to u64 are no longer needed.
Addresses-Coverity-ID: 1324146 ("Unintentional integer overflow")
Fixes 84a15ded76ec ("[media] V4L: Add driver for OV9650/52 image sensors")Fixes 79211c8ed19c ("remove abs64()")Signed-off-by Gustavo A. R. Silva <[email protected]>---
Changes in v2:
- Update subject and changelog to better reflect the proposed code changes.
- Add suffix ULL to constants instead of casting variables.
- Remove unnecessary casts to u64 as part of the code change.
- Extend the same code change to other similar expressions.Changes in v3:
- None.drivers/media/i2c/ov9650.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)Quoted 15 lines
drivers/media/i2c/ov9650.c static int __ov965x_set_frame_interval(struct ov965x *ov965x, · line 1134
Quoted 10 lines
drivers/media/i2c/ov9650.c +1
-0@@ -1130,7 +1130,7 @@ static int __ov965x_set_frame_interval(struct ov965x *ov965x, if (fi->interval.denominator == 0) return -EINVAL; - req_int = (u64)(fi->interval.numerator * 10000) / + req_int = fi->interval.numerator * 10000ULL / fi->interval.denominator;This has been addressed by your earlier patch "i2c: ov9650: fix potential integer overflow in
__ov965x_set_frame_interval" I tweaked a little. It's not in media tree
master yet.Yeah. Actually this patch is supposed to be an improved version of the one you mention. That is why this is version 3.
Also, I wonder if the same issue you mention below regarding 32-bit ARM applies in this case too?
Quoted 10 lines
drivers/media/i2c/ov9650.c static int __ov965x_set_frame_interval(struct ov965x *ov965x, · line 1142
Quoted 7 lines
drivers/media/i2c/ov9650.c +1
-1for (i = 0; i < ARRAY_SIZE(ov965x_intervals); i++) {@@ -1139,7 +1139,7 @@ static int __ov965x_set_frame_interval(struct ov965x *ov965x, if (mbus_fmt->width != iv->size.width || mbus_fmt->height != iv->size.height) continue; - err = abs((u64)(iv->interval.numerator * 10000) / + err = abs(iv->interval.numerator * 10000ULL /This and the chunk below won't work on e.g. 32-bit ARM.
do_div(), please.Thanks for pointing this out.
Quoted 13 lines
Quoted 11 lines
drivers/media/i2c/ov9650.c static int __ov965x_set_frame_interval(struct ov965x *ov965x, · line 1153
Quoted 11 lines
drivers/media/i2c/ov9650.c +3
-1iv->interval.denominator - req_int); if (err < min_err) { fiv = iv;@@ -1148,8 +1148,9 @@ static int __ov965x_set_frame_interval(struct ov965x *ov965x, } ov965x->fiv = fiv; - v4l2_dbg(1, debug, &ov965x->sd, "Changed frame interval to %u us\n", - fiv->interval.numerator * 1000000 / fiv->interval.denominator); + v4l2_dbg(1, debug, &ov965x->sd, "Changed frame interval to %llu us\n", + fiv->interval.numerator * 1000000ULL / + fiv->interval.denominator);I wonder if
do_divshould be used for the code above?
Quoted 2 lines
Yes,
do_divshould be used.
I got it.
Thanks, Hans.
--
Gustavo
PATCH v3 5/8 pci: cx88-input: use 64-bit arithmetic instead of 32-bit
In reply to Gustavo A. R. SilvaAdd suffix LL to constant 1000000 in order to give the compiler complete information about the proper arithmetic to use. Notice that this constant is used in a context that expects an expression of type ktime_t (64 bits, signed).
The expression ir->polling * 1000000 is currently being evaluated using 32-bit arithmetic.
Addresses-Coverity-ID: 1392628 ("Unintentional integer overflow")
Addresses-Coverity-ID: 1392630 ("Unintentional integer overflow")
---
Changes in v2:
- Update subject and changelog to better reflect the proposed code changes.
- Add suffix LL to constant instead of casting a variable.
Changes in v3:
- Mention the specific Coverity reports in the commit message.
drivers/media/pci/cx88/cx88-input.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/media/pci/cx88/cx88-input.c +2 -2
@@ -180,7 +180,7 @@ static enum hrtimer_restart cx88_ir_work(struct hrtimer *timer) struct cx88_IR *ir = container_of(timer, struct cx88_IR, timer);cx88_ir_handle_key (ir); - missed =hrtimer_forward_now (&ir->timer, ir->polling *1000000 ); + missed =hrtimer_forward_now (&ir->timer, ir->polling *1000000LL );if (missed >1 )ir_dprintk ("Missed ticks %ld\n" , missed -1 );
@@ -200,7 +200,7 @@ static int __cx88_ir_start(void *priv)if (ir->polling) {hrtimer_init (&ir->timer,CLOCK_MONOTONIC ,HRTIMER_MODE_REL ); ir->timer.function = cx88_ir_work; -hrtimer_start (&ir->timer, ir->polling *1000000 , +hrtimer_start (&ir->timer, ir->polling *1000000LL ,HRTIMER_MODE_REL ); }if (ir->sampling) {
Signature · 1 line
2.7.4
PATCH v3 6/8 rockchip/rga: use 64-bit arithmetic instead of 32-bit
In reply to Gustavo A. R. SilvaCast p to dma_addr_t in order to avoid a potential integer overflow. This variable is being used in a context that expects an expression of type dma_addr_t (u64).
The expression p << PAGE_SHIFT is currently being evaluated using 32-bit arithmetic.
Addresses-Coverity-ID: 1458347 ("Unintentional integer overflow")
---
Changes in v2:
- Update subject and changelog to better reflect the proposed code change.
Changes in v3:
- Mention the specific Coverity report in the commit message.
drivers/media/platform/rockchip/rga/rga-buf.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/media/platform/rockchip/rga/rga-buf.c +2 -1
@@ -140,7 +140,8 @@ void rga_buf_map(struct vb2_buffer *vb) address =sg_phys (sgl);for (p =0 ; p < len; p++) { -dma_addr_t phys = address + (p <<PAGE_SHIFT ); +dma_addr_t phys = address + + ((dma_addr_t )p <<PAGE_SHIFT ); pages[mapped_size + p] = phys; }
Signature · 1 line
2.7.4
PATCH v3 7/8 platform: sh_veu: use 64-bit arithmetic instead of 32-bit
In reply to Gustavo A. R. SilvaCast left and top to dma_addr_t in order to give the compiler complete information about the proper arithmetic to use. Notice that these variables are being used in contexts that expect expressions of type dma_addr_t (64 bit, unsigned).
Such expressions are currently being evaluated using 32-bit arithmetic.
Also, move the expression (((dma_addr_t)left * veu->vfmt_out.fmt->depth) >> 3) at the end in order to avoid a line wrapping checkpatch.pl warning.
Addresses-Coverity-ID: 1056807 ("Unintentional integer overflow")
Addresses-Coverity-ID: 1056808 ("Unintentional integer overflow")
---
Changes in v2:
- Update subject and changelog to better reflect the proposed code changes.
- Move the expression (((dma_addr_t)left * veu->vfmt_out.fmt->depth) >> 3) at the end in order to avoid a line wrapping checkpatch.pl warning.
Changes in v3:
- Mention the specific Coverity reports in the commit message.
drivers/media/platform/sh_veu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/media/platform/sh_veu.c +2 -2
@@ -520,8 +520,8 @@ static void sh_veu_colour_offset(struct sh_veu_dev *veu, struct sh_veu_vfmt *vfm/* dst_left and dst_top validity will be verified in CROP / COMPOSE */ unsigned int left = vfmt->frame.left & ~0x03 ;unsigned int top = vfmt->frame.top; -dma_addr_t offset = ((left * veu->vfmt_out.fmt->depth) >>3 ) + - top * veu->vfmt_out.bytesperline; +dma_addr_t offset = (dma_addr_t )top * veu->vfmt_out.bytesperline + + (((dma_addr_t )left * veu->vfmt_out.fmt->depth) >>3 );unsigned int y_line; vfmt->offset_y = offset;
Signature · 1 line
2.7.4
PATCH v3 8/8 platform: vivid-cec: use 64-bit arithmetic instead of 32-bit
In reply to Gustavo A. R. SilvaAdd suffix ULL to constant 10 in order to give the compiler complete information about the proper arithmetic to use. Notice that this constant is used in a context that expects an expression of type u64 (64 bits, unsigned).
The expression len * 10 * CEC_TIM_DATA_BIT_TOTAL is currently being evaluated using 32-bit arithmetic.
Also, remove unnecessary parentheses and add a code comment to make it clear what is the reason of the code change.
Addresses-Coverity-ID: 1454996 ("Unintentional integer overflow")
---
Changes in v2:
- Update subject and changelog to better reflect the proposed code changes.
- Add suffix ULL to constant instead of casting a variable.
- Remove unnecessary parentheses.
- Add code comment.
Changes in v3:
- Mention the specific Coverity report in the commit message.
drivers/media/platform/vivid/vivid-cec.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
drivers/media/platform/vivid/vivid-cec.c +9 -2
@@ -82,8 +82,15 @@ static void vivid_cec_pin_adap_events(struct cec_adapter *adap, ktime_t ts,if (adap ==NULL )return ; - ts =ktime_sub_us (ts, (CEC_TIM_START_BIT_TOTAL + - len *10 *CEC_TIM_DATA_BIT_TOTAL )); + +/* +* Suffix ULL on constant 10 makes the expression +* CEC_TIM_START_BIT_TOTAL + 10ULL * len * CEC_TIM_DATA_BIT_TOTAL +* to be evaluated using 64-bit unsigned arithmetic (u64), which +* is what ktime_sub_us expects as second argument. +*/ + ts =ktime_sub_us (ts,CEC_TIM_START_BIT_TOTAL + +10ULL * len *CEC_TIM_DATA_BIT_TOTAL );cec_queue_pin_cec_event (adap, false, ts); ts =ktime_add_us (ts,CEC_TIM_START_BIT_LOW );cec_queue_pin_cec_event (adap, true, ts);
Signature · 1 line
2.7.4