Hi Cyril, On Sun, Aug 19, 2018 at 09:57:37PM +0200, Cyril Bonté wrote: > Well, the original comment is not valid. The behaviour is not to round up > every values but to round values to the nearest int.
Oh I'm pretty well aware of it thanks to your explanation. I just want to be certain we don't need to revisit this patch later for another corner case. > > > - tmout = MS_TO_TICKS((int)dtmout); > > > + /* round up the timeout to convert it to int */ > > > + tmout = MS_TO_TICKS((int)(dtmout + 0.5)); > > > > and here we convert 2147483647.5 to int, which becomes -2147483648. > > 2147483647.5, when cast to int, should become 2147483647. You're totally right, my bad. Indeed it rounds down, not to the closest one, so your patch is fine. > > Given that the double to int conversion already rounds values, we do not > > need the +0.5 so we can get rid of it. Alternatively, dtmout could be > > rounded up before the checks using round() but apparently this is not > > needed. > > > > I suspect that the +0.5 might have been placed here to ensure that no > > short timeout is rounded down to zero. > > Adding 0.5 is a trick to round positive values to the nearest int (it works > in most cases). For example : > 1234.1 will become 1234 > 1234.6 will become 1235 Oh don't get me wrong, I'm totally aware of this and I do use it as well, it's just that for whatever reason I thought the cast to int would round as well, resulting in 2147483647.5 being turned to 2147483648. > We're talking about loosing 1ms of precision, so the rounding trick is maybe > an overkill feature. I'm fine with removing the 0.5 addition and adding 1ms > if the result is equal to 0. And I clearly prefer that behaviour ! Btw, this > can already happen for very small values. Currently, sock:settimeout(0.0001) > will result in a 0ms timeout. Yes, that's why I want to be sure that a small non-zero timeout results in a non-zero timeout :-) > > Just let me know if you want me to adjust the patch one way or another > > so that you don't need to respin one. > > I'm going to send you a new patch, which actually consists in including what > you've suggested ;-) OK thank you! Willy
