* [PATCH] iio: imu: adis16480: clean up a condition @ 2019-09-26 8:10 ` Dan Carpenter 0 siblings, 0 replies; 14+ messages in thread From: Dan Carpenter @ 2019-09-26 8:10 UTC (permalink / raw) To: Lars-Peter Clausen Cc: Michael Hennerich, Stefan Popa, Jonathan Cameron, Hartmut Knaack, Peter Meerwald-Stadler, linux-iio, linux-kernel, kernel-janitors The "t" variable is unsigned so it can't be less than zero. We really are just trying to prevent divide by zero bugs so just checking against zero is sufficient. Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- drivers/iio/imu/adis16480.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c index b99d73887c9f..e144e567675d 100644 --- a/drivers/iio/imu/adis16480.c +++ b/drivers/iio/imu/adis16480.c @@ -318,7 +318,7 @@ static int adis16480_set_freq(struct iio_dev *indio_dev, int val, int val2) unsigned int t, reg; t = val * 1000 + val2 / 1000; - if (t <= 0) + if (t == 0) return -EINVAL; /* -- 2.20.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH] iio: imu: adis16480: clean up a condition @ 2019-09-26 8:10 ` Dan Carpenter 0 siblings, 0 replies; 14+ messages in thread From: Dan Carpenter @ 2019-09-26 8:10 UTC (permalink / raw) To: Lars-Peter Clausen Cc: Michael Hennerich, Stefan Popa, Jonathan Cameron, Hartmut Knaack, Peter Meerwald-Stadler, linux-iio, linux-kernel, kernel-janitors The "t" variable is unsigned so it can't be less than zero. We really are just trying to prevent divide by zero bugs so just checking against zero is sufficient. Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- drivers/iio/imu/adis16480.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c index b99d73887c9f..e144e567675d 100644 --- a/drivers/iio/imu/adis16480.c +++ b/drivers/iio/imu/adis16480.c @@ -318,7 +318,7 @@ static int adis16480_set_freq(struct iio_dev *indio_dev, int val, int val2) unsigned int t, reg; t = val * 1000 + val2 / 1000; - if (t <= 0) + if (t = 0) return -EINVAL; /* -- 2.20.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] iio: imu: adis16480: clean up a condition 2019-09-26 8:10 ` Dan Carpenter @ 2019-09-26 11:06 ` Ardelean, Alexandru -1 siblings, 0 replies; 14+ messages in thread From: Ardelean, Alexandru @ 2019-09-26 11:06 UTC (permalink / raw) To: dan.carpenter, lars Cc: kernel-janitors, Popa, Stefan Serban, linux-iio, jic23, linux-kernel, Hennerich, Michael, pmeerw, knaack.h On Thu, 2019-09-26 at 11:10 +0300, Dan Carpenter wrote: > [External] > > The "t" variable is unsigned so it can't be less than zero. We really > are just trying to prevent divide by zero bugs so just checking against > zero is sufficient. > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > drivers/iio/imu/adis16480.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c > index b99d73887c9f..e144e567675d 100644 > --- a/drivers/iio/imu/adis16480.c > +++ b/drivers/iio/imu/adis16480.c > @@ -318,7 +318,7 @@ static int adis16480_set_freq(struct iio_dev > *indio_dev, int val, int val2) > unsigned int t, reg; I would just change the type of "t" to "int". Especially, since "val" & "val2" are "int". Thanks for the catch :) Alex > > t = val * 1000 + val2 / 1000; > - if (t <= 0) > + if (t == 0) > return -EINVAL; > > /* ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] iio: imu: adis16480: clean up a condition @ 2019-09-26 11:06 ` Ardelean, Alexandru 0 siblings, 0 replies; 14+ messages in thread From: Ardelean, Alexandru @ 2019-09-26 11:06 UTC (permalink / raw) To: dan.carpenter, lars Cc: kernel-janitors, Popa, Stefan Serban, linux-iio, jic23, linux-kernel, Hennerich, Michael, pmeerw, knaack.h T24gVGh1LCAyMDE5LTA5LTI2IGF0IDExOjEwICswMzAwLCBEYW4gQ2FycGVudGVyIHdyb3RlOg0K PiBbRXh0ZXJuYWxdDQo+IA0KPiBUaGUgInQiIHZhcmlhYmxlIGlzIHVuc2lnbmVkIHNvIGl0IGNh bid0IGJlIGxlc3MgdGhhbiB6ZXJvLiAgV2UgcmVhbGx5DQo+IGFyZSBqdXN0IHRyeWluZyB0byBw cmV2ZW50IGRpdmlkZSBieSB6ZXJvIGJ1Z3Mgc28ganVzdCBjaGVja2luZyBhZ2FpbnN0DQo+IHpl cm8gaXMgc3VmZmljaWVudC4NCj4gDQo+IFNpZ25lZC1vZmYtYnk6IERhbiBDYXJwZW50ZXIgPGRh bi5jYXJwZW50ZXJAb3JhY2xlLmNvbT4NCj4gLS0tDQo+ICBkcml2ZXJzL2lpby9pbXUvYWRpczE2 NDgwLmMgfCAyICstDQo+ICAxIGZpbGUgY2hhbmdlZCwgMSBpbnNlcnRpb24oKyksIDEgZGVsZXRp b24oLSkNCj4gDQo+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2lpby9pbXUvYWRpczE2NDgwLmMgYi9k cml2ZXJzL2lpby9pbXUvYWRpczE2NDgwLmMNCj4gaW5kZXggYjk5ZDczODg3YzlmLi5lMTQ0ZTU2 NzY3NWQgMTAwNjQ0DQo+IC0tLSBhL2RyaXZlcnMvaWlvL2ltdS9hZGlzMTY0ODAuYw0KPiArKysg Yi9kcml2ZXJzL2lpby9pbXUvYWRpczE2NDgwLmMNCj4gQEAgLTMxOCw3ICszMTgsNyBAQCBzdGF0 aWMgaW50IGFkaXMxNjQ4MF9zZXRfZnJlcShzdHJ1Y3QgaWlvX2Rldg0KPiAqaW5kaW9fZGV2LCBp bnQgdmFsLCBpbnQgdmFsMikNCj4gIAl1bnNpZ25lZCBpbnQgdCwgcmVnOw0KDQpJIHdvdWxkIGp1 c3QgY2hhbmdlIHRoZSB0eXBlIG9mICJ0IiB0byAiaW50Ii4NCkVzcGVjaWFsbHksIHNpbmNlICJ2 YWwiICYgInZhbDIiIGFyZSAiaW50Ii4NCg0KVGhhbmtzIGZvciB0aGUgY2F0Y2ggOikNCkFsZXgN Cg0KPiAgDQo+ICAJdCA9ICB2YWwgKiAxMDAwICsgdmFsMiAvIDEwMDA7DQo+IC0JaWYgKHQgPD0g MCkNCj4gKwlpZiAodCA9PSAwKQ0KPiAgCQlyZXR1cm4gLUVJTlZBTDsNCj4gIA0KPiAgCS8qDQo ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] iio: imu: adis16480: clean up a condition 2019-09-26 11:06 ` Ardelean, Alexandru @ 2019-09-26 11:36 ` Dan Carpenter -1 siblings, 0 replies; 14+ messages in thread From: Dan Carpenter @ 2019-09-26 11:36 UTC (permalink / raw) To: Ardelean, Alexandru Cc: lars, kernel-janitors, Popa, Stefan Serban, linux-iio, jic23, linux-kernel, Hennerich, Michael, pmeerw, knaack.h On Thu, Sep 26, 2019 at 11:06:39AM +0000, Ardelean, Alexandru wrote: > On Thu, 2019-09-26 at 11:10 +0300, Dan Carpenter wrote: > > [External] > > > > The "t" variable is unsigned so it can't be less than zero. We really > > are just trying to prevent divide by zero bugs so just checking against > > zero is sufficient. > > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > --- > > drivers/iio/imu/adis16480.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c > > index b99d73887c9f..e144e567675d 100644 > > --- a/drivers/iio/imu/adis16480.c > > +++ b/drivers/iio/imu/adis16480.c > > @@ -318,7 +318,7 @@ static int adis16480_set_freq(struct iio_dev > > *indio_dev, int val, int val2) > > unsigned int t, reg; > > I would just change the type of "t" to "int". > Especially, since "val" & "val2" are "int". > Yeah, but negatives or high values are basically fine. We only care zero. regards, dan carpenter ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] iio: imu: adis16480: clean up a condition @ 2019-09-26 11:36 ` Dan Carpenter 0 siblings, 0 replies; 14+ messages in thread From: Dan Carpenter @ 2019-09-26 11:36 UTC (permalink / raw) To: Ardelean, Alexandru Cc: lars, kernel-janitors, Popa, Stefan Serban, linux-iio, jic23, linux-kernel, Hennerich, Michael, pmeerw, knaack.h On Thu, Sep 26, 2019 at 11:06:39AM +0000, Ardelean, Alexandru wrote: > On Thu, 2019-09-26 at 11:10 +0300, Dan Carpenter wrote: > > [External] > > > > The "t" variable is unsigned so it can't be less than zero. We really > > are just trying to prevent divide by zero bugs so just checking against > > zero is sufficient. > > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > --- > > drivers/iio/imu/adis16480.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c > > index b99d73887c9f..e144e567675d 100644 > > --- a/drivers/iio/imu/adis16480.c > > +++ b/drivers/iio/imu/adis16480.c > > @@ -318,7 +318,7 @@ static int adis16480_set_freq(struct iio_dev > > *indio_dev, int val, int val2) > > unsigned int t, reg; > > I would just change the type of "t" to "int". > Especially, since "val" & "val2" are "int". > Yeah, but negatives or high values are basically fine. We only care zero. regards, dan carpenter ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] iio: imu: adis16480: clean up a condition 2019-09-26 11:36 ` Dan Carpenter @ 2019-10-06 8:51 ` Jonathan Cameron -1 siblings, 0 replies; 14+ messages in thread From: Jonathan Cameron @ 2019-10-06 8:51 UTC (permalink / raw) To: Dan Carpenter Cc: Ardelean, Alexandru, lars, kernel-janitors, Popa, Stefan Serban, linux-iio, linux-kernel, Hennerich, Michael, pmeerw, knaack.h On Thu, 26 Sep 2019 14:36:30 +0300 Dan Carpenter <dan.carpenter@oracle.com> wrote: > On Thu, Sep 26, 2019 at 11:06:39AM +0000, Ardelean, Alexandru wrote: > > On Thu, 2019-09-26 at 11:10 +0300, Dan Carpenter wrote: > > > [External] > > > > > > The "t" variable is unsigned so it can't be less than zero. We really > > > are just trying to prevent divide by zero bugs so just checking against > > > zero is sufficient. I'm not sure that true. It if were signed we'd be detecting that the input from userspace was negative. That is clearly garbage for a frequency control.. I'll hazard a guess that it was intended to catch that as well as the divide by 0 case. This would be clearer if we first checked that val and val2 are both not negative, then did the zero test on t. If people agree, anyone want to spin a patch to do that? Thanks, Jonathan > > > > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > > --- > > > drivers/iio/imu/adis16480.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c > > > index b99d73887c9f..e144e567675d 100644 > > > --- a/drivers/iio/imu/adis16480.c > > > +++ b/drivers/iio/imu/adis16480.c > > > @@ -318,7 +318,7 @@ static int adis16480_set_freq(struct iio_dev > > > *indio_dev, int val, int val2) > > > unsigned int t, reg; > > > > I would just change the type of "t" to "int". > > Especially, since "val" & "val2" are "int". > > > > Yeah, but negatives or high values are basically fine. We only care > zero. > > regards, > dan carpenter > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] iio: imu: adis16480: clean up a condition @ 2019-10-06 8:51 ` Jonathan Cameron 0 siblings, 0 replies; 14+ messages in thread From: Jonathan Cameron @ 2019-10-06 8:51 UTC (permalink / raw) To: Dan Carpenter Cc: Ardelean, Alexandru, lars, kernel-janitors, Popa, Stefan Serban, linux-iio, linux-kernel, Hennerich, Michael, pmeerw, knaack.h On Thu, 26 Sep 2019 14:36:30 +0300 Dan Carpenter <dan.carpenter@oracle.com> wrote: > On Thu, Sep 26, 2019 at 11:06:39AM +0000, Ardelean, Alexandru wrote: > > On Thu, 2019-09-26 at 11:10 +0300, Dan Carpenter wrote: > > > [External] > > > > > > The "t" variable is unsigned so it can't be less than zero. We really > > > are just trying to prevent divide by zero bugs so just checking against > > > zero is sufficient. I'm not sure that true. It if were signed we'd be detecting that the input from userspace was negative. That is clearly garbage for a frequency control.. I'll hazard a guess that it was intended to catch that as well as the divide by 0 case. This would be clearer if we first checked that val and val2 are both not negative, then did the zero test on t. If people agree, anyone want to spin a patch to do that? Thanks, Jonathan > > > > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > > --- > > > drivers/iio/imu/adis16480.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c > > > index b99d73887c9f..e144e567675d 100644 > > > --- a/drivers/iio/imu/adis16480.c > > > +++ b/drivers/iio/imu/adis16480.c > > > @@ -318,7 +318,7 @@ static int adis16480_set_freq(struct iio_dev > > > *indio_dev, int val, int val2) > > > unsigned int t, reg; > > > > I would just change the type of "t" to "int". > > Especially, since "val" & "val2" are "int". > > > > Yeah, but negatives or high values are basically fine. We only care > zero. > > regards, > dan carpenter > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] iio: imu: adis16480: clean up a condition 2019-10-06 8:51 ` Jonathan Cameron @ 2019-10-06 18:14 ` Dan Carpenter -1 siblings, 0 replies; 14+ messages in thread From: Dan Carpenter @ 2019-10-06 18:14 UTC (permalink / raw) To: Jonathan Cameron Cc: Ardelean, Alexandru, lars, kernel-janitors, Popa, Stefan Serban, linux-iio, linux-kernel, Hennerich, Michael, pmeerw, knaack.h On Sun, Oct 06, 2019 at 09:51:33AM +0100, Jonathan Cameron wrote: > On Thu, 26 Sep 2019 14:36:30 +0300 > Dan Carpenter <dan.carpenter@oracle.com> wrote: > > > On Thu, Sep 26, 2019 at 11:06:39AM +0000, Ardelean, Alexandru wrote: > > > On Thu, 2019-09-26 at 11:10 +0300, Dan Carpenter wrote: > > > > [External] > > > > > > > > The "t" variable is unsigned so it can't be less than zero. We really > > > > are just trying to prevent divide by zero bugs so just checking against > > > > zero is sufficient. > > I'm not sure that true. It if were signed we'd be detecting that the > input from userspace was negative. It does a really bad job of that though so it raises more questions than answers. Maybe just one of the parameters is negative or maybe the multiply or the addition overflowed? Should scenarios those be checked? It turns out none of those situations matter, only divide by zero needs to be checked. regards, dan carpenter ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] iio: imu: adis16480: clean up a condition @ 2019-10-06 18:14 ` Dan Carpenter 0 siblings, 0 replies; 14+ messages in thread From: Dan Carpenter @ 2019-10-06 18:14 UTC (permalink / raw) To: Jonathan Cameron Cc: Ardelean, Alexandru, lars, kernel-janitors, Popa, Stefan Serban, linux-iio, linux-kernel, Hennerich, Michael, pmeerw, knaack.h On Sun, Oct 06, 2019 at 09:51:33AM +0100, Jonathan Cameron wrote: > On Thu, 26 Sep 2019 14:36:30 +0300 > Dan Carpenter <dan.carpenter@oracle.com> wrote: > > > On Thu, Sep 26, 2019 at 11:06:39AM +0000, Ardelean, Alexandru wrote: > > > On Thu, 2019-09-26 at 11:10 +0300, Dan Carpenter wrote: > > > > [External] > > > > > > > > The "t" variable is unsigned so it can't be less than zero. We really > > > > are just trying to prevent divide by zero bugs so just checking against > > > > zero is sufficient. > > I'm not sure that true. It if were signed we'd be detecting that the > input from userspace was negative. It does a really bad job of that though so it raises more questions than answers. Maybe just one of the parameters is negative or maybe the multiply or the addition overflowed? Should scenarios those be checked? It turns out none of those situations matter, only divide by zero needs to be checked. regards, dan carpenter ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] iio: imu: adis16480: clean up a condition 2019-10-06 18:14 ` Dan Carpenter @ 2019-10-07 9:21 ` Jonathan Cameron -1 siblings, 0 replies; 14+ messages in thread From: Jonathan Cameron @ 2019-10-07 9:21 UTC (permalink / raw) To: Dan Carpenter Cc: Jonathan Cameron, Ardelean, Alexandru, lars, kernel-janitors, Popa, Stefan Serban, linux-iio, linux-kernel, Hennerich, Michael, pmeerw, knaack.h On Sun, 6 Oct 2019 21:14:40 +0300 Dan Carpenter <dan.carpenter@oracle.com> wrote: > On Sun, Oct 06, 2019 at 09:51:33AM +0100, Jonathan Cameron wrote: > > On Thu, 26 Sep 2019 14:36:30 +0300 > > Dan Carpenter <dan.carpenter@oracle.com> wrote: > > > > > On Thu, Sep 26, 2019 at 11:06:39AM +0000, Ardelean, Alexandru wrote: > > > > On Thu, 2019-09-26 at 11:10 +0300, Dan Carpenter wrote: > > > > > [External] > > > > > > > > > > The "t" variable is unsigned so it can't be less than zero. We really > > > > > are just trying to prevent divide by zero bugs so just checking against > > > > > zero is sufficient. > > > > I'm not sure that true. It if were signed we'd be detecting that the > > input from userspace was negative. > > It does a really bad job of that though so it raises more questions than > answers. Maybe just one of the parameters is negative or maybe the > multiply or the addition overflowed? Should scenarios those be checked? > > It turns out none of those situations matter, only divide by zero needs > to be checked. It isn't being nearly paranoid enough. Either val or val2 being negative is a reason to fault out. Divide by zero needs handling after that. Obviously divide by zero is the only one that causes a crash but negatives are going to cause rather 'unexpected' results. What fun. Jonathan ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] iio: imu: adis16480: clean up a condition @ 2019-10-07 9:21 ` Jonathan Cameron 0 siblings, 0 replies; 14+ messages in thread From: Jonathan Cameron @ 2019-10-07 9:21 UTC (permalink / raw) To: Dan Carpenter Cc: Jonathan Cameron, Ardelean, Alexandru, lars, kernel-janitors, Popa, Stefan Serban, linux-iio, linux-kernel, Hennerich, Michael, pmeerw, knaack.h On Sun, 6 Oct 2019 21:14:40 +0300 Dan Carpenter <dan.carpenter@oracle.com> wrote: > On Sun, Oct 06, 2019 at 09:51:33AM +0100, Jonathan Cameron wrote: > > On Thu, 26 Sep 2019 14:36:30 +0300 > > Dan Carpenter <dan.carpenter@oracle.com> wrote: > > > > > On Thu, Sep 26, 2019 at 11:06:39AM +0000, Ardelean, Alexandru wrote: > > > > On Thu, 2019-09-26 at 11:10 +0300, Dan Carpenter wrote: > > > > > [External] > > > > > > > > > > The "t" variable is unsigned so it can't be less than zero. We really > > > > > are just trying to prevent divide by zero bugs so just checking against > > > > > zero is sufficient. > > > > I'm not sure that true. It if were signed we'd be detecting that the > > input from userspace was negative. > > It does a really bad job of that though so it raises more questions than > answers. Maybe just one of the parameters is negative or maybe the > multiply or the addition overflowed? Should scenarios those be checked? > > It turns out none of those situations matter, only divide by zero needs > to be checked. It isn't being nearly paranoid enough. Either val or val2 being negative is a reason to fault out. Divide by zero needs handling after that. Obviously divide by zero is the only one that causes a crash but negatives are going to cause rather 'unexpected' results. What fun. Jonathan ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] iio: imu: adis16480: clean up a condition 2019-10-07 9:21 ` Jonathan Cameron @ 2019-10-07 14:18 ` Dan Carpenter -1 siblings, 0 replies; 14+ messages in thread From: Dan Carpenter @ 2019-10-07 14:18 UTC (permalink / raw) To: Jonathan Cameron Cc: Jonathan Cameron, Ardelean, Alexandru, lars, kernel-janitors, Popa, Stefan Serban, linux-iio, linux-kernel, Hennerich, Michael, pmeerw, knaack.h On Mon, Oct 07, 2019 at 10:21:07AM +0100, Jonathan Cameron wrote: > On Sun, 6 Oct 2019 21:14:40 +0300 > Dan Carpenter <dan.carpenter@oracle.com> wrote: > > > On Sun, Oct 06, 2019 at 09:51:33AM +0100, Jonathan Cameron wrote: > > > On Thu, 26 Sep 2019 14:36:30 +0300 > > > Dan Carpenter <dan.carpenter@oracle.com> wrote: > > > > > > > On Thu, Sep 26, 2019 at 11:06:39AM +0000, Ardelean, Alexandru wrote: > > > > > On Thu, 2019-09-26 at 11:10 +0300, Dan Carpenter wrote: > > > > > > [External] > > > > > > > > > > > > The "t" variable is unsigned so it can't be less than zero. We really > > > > > > are just trying to prevent divide by zero bugs so just checking against > > > > > > zero is sufficient. > > > > > > I'm not sure that true. It if were signed we'd be detecting that the > > > input from userspace was negative. > > > > It does a really bad job of that though so it raises more questions than > > answers. Maybe just one of the parameters is negative or maybe the > > multiply or the addition overflowed? Should scenarios those be checked? > > > > It turns out none of those situations matter, only divide by zero needs > > to be checked. > > It isn't being nearly paranoid enough. Either val or val2 being > negative is a reason to fault out. Divide by zero needs handling after > that. Obviously divide by zero is the only one that causes a crash but > negatives are going to cause rather 'unexpected' results. The result is that it gets capped at st->chip_info->max_dec_rate so it's not a horrible result. :P I don't know if it's unexpected or not. regards, dan carpenter ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] iio: imu: adis16480: clean up a condition @ 2019-10-07 14:18 ` Dan Carpenter 0 siblings, 0 replies; 14+ messages in thread From: Dan Carpenter @ 2019-10-07 14:18 UTC (permalink / raw) To: Jonathan Cameron Cc: Jonathan Cameron, Ardelean, Alexandru, lars, kernel-janitors, Popa, Stefan Serban, linux-iio, linux-kernel, Hennerich, Michael, pmeerw, knaack.h On Mon, Oct 07, 2019 at 10:21:07AM +0100, Jonathan Cameron wrote: > On Sun, 6 Oct 2019 21:14:40 +0300 > Dan Carpenter <dan.carpenter@oracle.com> wrote: > > > On Sun, Oct 06, 2019 at 09:51:33AM +0100, Jonathan Cameron wrote: > > > On Thu, 26 Sep 2019 14:36:30 +0300 > > > Dan Carpenter <dan.carpenter@oracle.com> wrote: > > > > > > > On Thu, Sep 26, 2019 at 11:06:39AM +0000, Ardelean, Alexandru wrote: > > > > > On Thu, 2019-09-26 at 11:10 +0300, Dan Carpenter wrote: > > > > > > [External] > > > > > > > > > > > > The "t" variable is unsigned so it can't be less than zero. We really > > > > > > are just trying to prevent divide by zero bugs so just checking against > > > > > > zero is sufficient. > > > > > > I'm not sure that true. It if were signed we'd be detecting that the > > > input from userspace was negative. > > > > It does a really bad job of that though so it raises more questions than > > answers. Maybe just one of the parameters is negative or maybe the > > multiply or the addition overflowed? Should scenarios those be checked? > > > > It turns out none of those situations matter, only divide by zero needs > > to be checked. > > It isn't being nearly paranoid enough. Either val or val2 being > negative is a reason to fault out. Divide by zero needs handling after > that. Obviously divide by zero is the only one that causes a crash but > negatives are going to cause rather 'unexpected' results. The result is that it gets capped at st->chip_info->max_dec_rate so it's not a horrible result. :P I don't know if it's unexpected or not. regards, dan carpenter ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-10-07 14:18 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-09-26 8:10 [PATCH] iio: imu: adis16480: clean up a condition Dan Carpenter 2019-09-26 8:10 ` Dan Carpenter 2019-09-26 11:06 ` Ardelean, Alexandru 2019-09-26 11:06 ` Ardelean, Alexandru 2019-09-26 11:36 ` Dan Carpenter 2019-09-26 11:36 ` Dan Carpenter 2019-10-06 8:51 ` Jonathan Cameron 2019-10-06 8:51 ` Jonathan Cameron 2019-10-06 18:14 ` Dan Carpenter 2019-10-06 18:14 ` Dan Carpenter 2019-10-07 9:21 ` Jonathan Cameron 2019-10-07 9:21 ` Jonathan Cameron 2019-10-07 14:18 ` Dan Carpenter 2019-10-07 14:18 ` Dan Carpenter
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.