On Thu, Nov 1, 2018 at 5:22 AM Johannes Sixt <j6t@xxxxxxxx> wrote: > > Am 31.10.18 um 22:11 schrieb Steve Hoelzer via GitGitGadget: > > From: Steve Hoelzer <shoelzer@xxxxxxxxx> > > > > From Visual Studio 2015 Code Analysis: Warning C28159 Consider using > > 'GetTickCount64' instead of 'GetTickCount'. > > > > Reason: GetTickCount() overflows roughly every 49 days. Code that does > > not take that into account can loop indefinitely. GetTickCount64() > > operates on 64 bit values and does not have that problem. > > > > Note: this patch has been carried in Git for Windows for almost two > > years, but with a fallback for Windows XP, as the GetTickCount64() > > function is only available on Windows Vista and later. However, in the > > meantime we require Vista or later, hence we can drop that fallback. > > > > Signed-off-by: Steve Hoelzer <shoelzer@xxxxxxxxx> > > Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx> > > --- > > compat/poll/poll.c | 10 +++++++--- > > 1 file changed, 7 insertions(+), 3 deletions(-) > > > > diff --git a/compat/poll/poll.c b/compat/poll/poll.c > > index ad5dcde439..4abbfcb6a4 100644 > > --- a/compat/poll/poll.c > > +++ b/compat/poll/poll.c > > @@ -18,6 +18,9 @@ > > You should have received a copy of the GNU General Public License along > > with this program; if not, see <http://www.gnu.org/licenses/>. */ > > > > +/* To bump the minimum Windows version to Windows Vista */ > > +#include "git-compat-util.h" > > + > > /* Tell gcc not to warn about the (nfd < 0) tests, below. */ > > #if (__GNUC__ == 4 && 3 <= __GNUC_MINOR__) || 4 < __GNUC__ > > # pragma GCC diagnostic ignored "-Wtype-limits" > > @@ -449,7 +452,8 @@ poll (struct pollfd *pfd, nfds_t nfd, int timeout) > > static HANDLE hEvent; > > WSANETWORKEVENTS ev; > > HANDLE h, handle_array[FD_SETSIZE + 2]; > > - DWORD ret, wait_timeout, nhandles, start = 0, elapsed, orig_timeout = 0; > > + DWORD ret, wait_timeout, nhandles, elapsed, orig_timeout = 0; > > + ULONGLONG start = 0; > > fd_set rfds, wfds, xfds; > > BOOL poll_again; > > MSG msg; > > @@ -465,7 +469,7 @@ poll (struct pollfd *pfd, nfds_t nfd, int timeout) > > if (timeout != INFTIM) > > { > > orig_timeout = timeout; > > - start = GetTickCount(); > > + start = GetTickCount64(); > > } > > > > if (!hEvent) > > @@ -614,7 +618,7 @@ restart: > > > > if (!rc && orig_timeout && timeout != INFTIM) > > { > > - elapsed = GetTickCount() - start; > > + elapsed = (DWORD)(GetTickCount64() - start); > > AFAICS, this subtraction in the old code is the correct way to take > account of wrap-arounds in the tick count. The new code truncates the 64 > bit difference to 32 bits; the result is exactly identical to a > difference computed from truncated 32 bit values, which is what we had > in the old code. > > IOW, there is no change in behavior. The statement "avoid wrap-around > issues" in the subject line is not correct. The patch's only effect is > that it removes Warning C28159. > > What is really needed is that all quantities in the calculations are > promoted to ULONGLONG. Unless, of course, we agree that a timeout of > more than 49 days cannot happen ;) Yep, correct on all counts. I'm in favor of changing the commit message to only say that this patch removes Warning C28159. Steve
