Re: [greybus-dev] [PATCH] staging: greybus: use proper return type for wait_for_completion_timeout
On Sat, Apr 27, 2019 at 05:27:25AM +0200, Nicholas Mc Guire wrote:
wait_for_completion_timeout() returns unsigned long (0 on timeout or remaining jiffies) not int.
Yeah, but it's fine though because 10000 / 256 fits into int without a problem.
I'm not sure this sort of patch is worth it when it's just a style debate instead of a bugfix. I'm a little bit torn about this. In Smatch, I run into this issue one in a while where Smatch doesn't know if the timeout is less than int. Right now I hacked the DB to say that these functions always return < INT_MAX.
Anyway, for sure the commit message should say that it's just a cleanup and not a bugfix.
regards, dan carpenter
On Tue, Apr 30, 2019 at 12:58:21PM +0300, Dan Carpenter wrote:
On Sat, Apr 27, 2019 at 05:27:25AM +0200, Nicholas Mc Guire wrote:
wait_for_completion_timeout() returns unsigned long (0 on timeout or remaining jiffies) not int.
Yeah, but it's fine though because 10000 / 256 fits into int without a problem.
I'm not sure this sort of patch is worth it when it's just a style debate instead of a bugfix. I'm a little bit torn about this. In Smatch, I run into this issue one in a while where Smatch doesn't know if the timeout is less than int. Right now I hacked the DB to say that these functions always return < INT_MAX.
Anyway, for sure the commit message should say that it's just a cleanup and not a bugfix.
I know its not a functional bug its "only" an API violation - the problem is more that code is often cut&past and at some point it may be a problem or someoe expects a negative return value without that this evef can occure.
But yes - the commit message should have stated that this non-conformance in this case has no effect - will resend.
thx! hofrat
-
Dan Carpenter -
Nicholas Mc Guire