{"thread":{"id":"49727","subject":"[PATCH 0/1] Make compat/poll safer on Windows","startedAt":"2018-10-31T21:11:38Z","lastAt":"2018-11-05T22:05:10Z","messageCount":17,"participants":["Johannes Schindelin via GitGitGadget","Steve Hoelzer via GitGitGadget","Johannes Sixt","Steve Hoelzer","Junio C Hamano","Carlo Arenas","Randall S. Becker","Eric Sunshine","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"362131","messageId":"pull.64.git.gitgitgadget@gmail.com","threadId":"49727","inReplyTo":null,"subject":"[PATCH 0/1] Make compat/poll safer on Windows","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-10-31T21:11:35Z","receivedAt":"2018-10-31T21:11:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"This is yet another piece from the Git for Windows cake. It avoids a\nwrap-around in the poll emulation on Windows that occurs every 49 days.\n\nSteve Hoelzer (1):\n poll: use GetTickCount64() to avoid wrap-around issues\n\n compat/poll/poll.c | 10 +++++++---\n 1 file changed, 7 insertions(+), 3 deletions(-)\n\n\nbase-commit: 4ede3d42dfb57f9a41ac96a1f216c62eb7566cc2\nPublished-As: https://github.com/gitgitgadget/git/releases/tags/pr-64%2Fdscho%2Fmingw-safer-compat-poll-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-64/dscho/mingw-safer-compat-poll-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/64\n-- \ngitgitgadget\n"},{"id":"362132","messageId":"69bc5924f94b56f92d9653b3a64f721bd03f1956.1541020294.git.gitgitgadget@gmail.com","threadId":"49727","inReplyTo":"pull.64.git.gitgitgadget@gmail.com","subject":"[PATCH 1/1] poll: use GetTickCount64() to avoid wrap-around issues","fromName":"Steve Hoelzer via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-10-31T21:11:36Z","receivedAt":"2018-10-31T21:11:40Z","isPatch":true,"sender":{"key":"shoelzer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/4623209?v=4"},"body":"From: Steve Hoelzer \n\nFrom Visual Studio 2015 Code Analysis: Warning C28159 Consider using\n'GetTickCount64' instead of 'GetTickCount'.\n\nReason: GetTickCount() overflows roughly every 49 days. Code that does\nnot take that into account can loop indefinitely. GetTickCount64()\noperates on 64 bit values and does not have that problem.\n\nNote: this patch has been carried in Git for Windows for almost two\nyears, but with a fallback for Windows XP, as the GetTickCount64()\nfunction is only available on Windows Vista and later. However, in the\nmeantime we require Vista or later, hence we can drop that fallback.\n\nSigned-off-by: Steve Hoelzer \nSigned-off-by: Johannes Schindelin \n---\n compat/poll/poll.c | 10 +++++++---\n 1 file changed, 7 insertions(+), 3 deletions(-)\n\ndiff --git a/compat/poll/poll.c b/compat/poll/poll.c\nindex ad5dcde439..4abbfcb6a4 100644\n--- a/compat/poll/poll.c\n+++ b/compat/poll/poll.c\n@@ -18,6 +18,9 @@\n You should have received a copy of the GNU General Public License along\n with this program; if not, see . */\n \n+/* To bump the minimum Windows version to Windows Vista */\n+#include \"git-compat-util.h\"\n+\n /* Tell gcc not to warn about the (nfd < 0) tests, below. */\n #if (__GNUC__ == 4 && 3 <= __GNUC_MINOR__) || 4 < __GNUC__\n # pragma GCC diagnostic ignored \"-Wtype-limits\"\n@@ -449,7 +452,8 @@ poll (struct pollfd *pfd, nfds_t nfd, int timeout)\n static HANDLE hEvent;\n WSANETWORKEVENTS ev;\n HANDLE h, handle_array[FD_SETSIZE + 2];\n- DWORD ret, wait_timeout, nhandles, start = 0, elapsed, orig_timeout = 0;\n+ DWORD ret, wait_timeout, nhandles, elapsed, orig_timeout = 0;\n+ ULONGLONG start = 0;\n fd_set rfds, wfds, xfds;\n BOOL poll_again;\n MSG msg;\n@@ -465,7 +469,7 @@ poll (struct pollfd *pfd, nfds_t nfd, int timeout)\n if (timeout != INFTIM)\n {\n orig_timeout = timeout;\n- start = GetTickCount();\n+ start = GetTickCount64();\n }\n \n if (!hEvent)\n@@ -614,7 +618,7 @@ restart:\n \n if (!rc && orig_timeout && timeout != INFTIM)\n {\n- elapsed = GetTickCount() - start;\n+ elapsed = (DWORD)(GetTickCount64() - start);\n timeout = elapsed >= orig_timeout ? 0 : orig_timeout - elapsed;\n }\n \n-- \ngitgitgadget\n"},{"id":"362158","messageId":"c9e001de-3598-182d-416e-1e94f234c249@kdbg.org","threadId":"49727","inReplyTo":"69bc5924f94b56f92d9653b3a64f721bd03f1956.1541020294.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] poll: use GetTickCount64() to avoid wrap-around issues","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2018-11-01T10:21:59Z","receivedAt":"2018-11-01T10:22:04Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 31.10.18 um 22:11 schrieb Steve Hoelzer via GitGitGadget:\n> From: Steve Hoelzer \n> \n> From Visual Studio 2015 Code Analysis: Warning C28159 Consider using\n> 'GetTickCount64' instead of 'GetTickCount'.\n> \n> Reason: GetTickCount() overflows roughly every 49 days. Code that does\n> not take that into account can loop indefinitely. GetTickCount64()\n> operates on 64 bit values and does not have that problem.\n> \n> Note: this patch has been carried in Git for Windows for almost two\n> years, but with a fallback for Windows XP, as the GetTickCount64()\n> function is only available on Windows Vista and later. However, in the\n> meantime we require Vista or later, hence we can drop that fallback.\n> \n> Signed-off-by: Steve Hoelzer \n> Signed-off-by: Johannes Schindelin \n> ---\n> compat/poll/poll.c | 10 +++++++---\n> 1 file changed, 7 insertions(+), 3 deletions(-)\n> \n> diff --git a/compat/poll/poll.c b/compat/poll/poll.c\n> index ad5dcde439..4abbfcb6a4 100644\n> --- a/compat/poll/poll.c\n> +++ b/compat/poll/poll.c\n> @@ -18,6 +18,9 @@\n> You should have received a copy of the GNU General Public License along\n> with this program; if not, see . */\n> \n> +/* To bump the minimum Windows version to Windows Vista */\n> +#include \"git-compat-util.h\"\n> +\n> /* Tell gcc not to warn about the (nfd < 0) tests, below. */\n> #if (__GNUC__ == 4 && 3 <= __GNUC_MINOR__) || 4 < __GNUC__\n> # pragma GCC diagnostic ignored \"-Wtype-limits\"\n> @@ -449,7 +452,8 @@ poll (struct pollfd *pfd, nfds_t nfd, int timeout)\n> static HANDLE hEvent;\n> WSANETWORKEVENTS ev;\n> HANDLE h, handle_array[FD_SETSIZE + 2];\n> - DWORD ret, wait_timeout, nhandles, start = 0, elapsed, orig_timeout = 0;\n> + DWORD ret, wait_timeout, nhandles, elapsed, orig_timeout = 0;\n> + ULONGLONG start = 0;\n> fd_set rfds, wfds, xfds;\n> BOOL poll_again;\n> MSG msg;\n> @@ -465,7 +469,7 @@ poll (struct pollfd *pfd, nfds_t nfd, int timeout)\n> if (timeout != INFTIM)\n> {\n> orig_timeout = timeout;\n> - start = GetTickCount();\n> + start = GetTickCount64();\n> }\n> \n> if (!hEvent)\n> @@ -614,7 +618,7 @@ restart:\n> \n> if (!rc && orig_timeout && timeout != INFTIM)\n> {\n> - elapsed = GetTickCount() - start;\n> + elapsed = (DWORD)(GetTickCount64() - start);\n\nAFAICS, this subtraction in the old code is the correct way to take \naccount of wrap-arounds in the tick count. The new code truncates the 64 \nbit difference to 32 bits; the result is exactly identical to a \ndifference computed from truncated 32 bit values, which is what we had \nin the old code.\n\nIOW, there is no change in behavior. The statement \"avoid wrap-around \nissues\" in the subject line is not correct. The patch's only effect is \nthat it removes Warning C28159.\n\nWhat is really needed is that all quantities in the calculations are \npromoted to ULONGLONG. Unless, of course, we agree that a timeout of \nmore than 49 days cannot happen ;)\n\n> timeout = elapsed >= orig_timeout ? 0 : orig_timeout - elapsed;\n> }\n> \n\n-- Hannes\n"},{"id":"362271","messageId":"CACbrTHctZejfDTjqWqVfPYdb=ssD253Cd2isr3BxWsL1AqsH2w@mail.gmail.com","threadId":"49727","inReplyTo":"c9e001de-3598-182d-416e-1e94f234c249@kdbg.org","subject":"Re: [PATCH 1/1] poll: use GetTickCount64() to avoid wrap-around issues","fromName":"Steve Hoelzer","fromEmail":"shoelzer@gmail.com","sentAt":"2018-11-02T14:47:15Z","receivedAt":"2018-11-02T14:47:50Z","isPatch":true,"sender":{"key":"shoelzer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/4623209?v=4"},"body":"On Thu, Nov 1, 2018 at 5:22 AM Johannes Sixt wrote:\n>\n> Am 31.10.18 um 22:11 schrieb Steve Hoelzer via GitGitGadget:\n> > From: Steve Hoelzer \n> >\n> > From Visual Studio 2015 Code Analysis: Warning C28159 Consider using\n> > 'GetTickCount64' instead of 'GetTickCount'.\n> >\n> > Reason: GetTickCount() overflows roughly every 49 days. Code that does\n> > not take that into account can loop indefinitely. GetTickCount64()\n> > operates on 64 bit values and does not have that problem.\n> >\n> > Note: this patch has been carried in Git for Windows for almost two\n> > years, but with a fallback for Windows XP, as the GetTickCount64()\n> > function is only available on Windows Vista and later. However, in the\n> > meantime we require Vista or later, hence we can drop that fallback.\n> >\n> > Signed-off-by: Steve Hoelzer \n> > Signed-off-by: Johannes Schindelin \n> > ---\n> > compat/poll/poll.c | 10 +++++++---\n> > 1 file changed, 7 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/compat/poll/poll.c b/compat/poll/poll.c\n> > index ad5dcde439..4abbfcb6a4 100644\n> > --- a/compat/poll/poll.c\n> > +++ b/compat/poll/poll.c\n> > @@ -18,6 +18,9 @@\n> > You should have received a copy of the GNU General Public License along\n> > with this program; if not, see . */\n> >\n> > +/* To bump the minimum Windows version to Windows Vista */\n> > +#include \"git-compat-util.h\"\n> > +\n> > /* Tell gcc not to warn about the (nfd < 0) tests, below. */\n> > #if (__GNUC__ == 4 && 3 <= __GNUC_MINOR__) || 4 < __GNUC__\n> > # pragma GCC diagnostic ignored \"-Wtype-limits\"\n> > @@ -449,7 +452,8 @@ poll (struct pollfd *pfd, nfds_t nfd, int timeout)\n> > static HANDLE hEvent;\n> > WSANETWORKEVENTS ev;\n> > HANDLE h, handle_array[FD_SETSIZE + 2];\n> > - DWORD ret, wait_timeout, nhandles, start = 0, elapsed, orig_timeout = 0;\n> > + DWORD ret, wait_timeout, nhandles, elapsed, orig_timeout = 0;\n> > + ULONGLONG start = 0;\n> > fd_set rfds, wfds, xfds;\n> > BOOL poll_again;\n> > MSG msg;\n> > @@ -465,7 +469,7 @@ poll (struct pollfd *pfd, nfds_t nfd, int timeout)\n> > if (timeout != INFTIM)\n> > {\n> > orig_timeout = timeout;\n> > - start = GetTickCount();\n> > + start = GetTickCount64();\n> > }\n> >\n> > if (!hEvent)\n> > @@ -614,7 +618,7 @@ restart:\n> >\n> > if (!rc && orig_timeout && timeout != INFTIM)\n> > {\n> > - elapsed = GetTickCount() - start;\n> > + elapsed = (DWORD)(GetTickCount64() - start);\n>\n> AFAICS, this subtraction in the old code is the correct way to take\n> account of wrap-arounds in the tick count. The new code truncates the 64\n> bit difference to 32 bits; the result is exactly identical to a\n> difference computed from truncated 32 bit values, which is what we had\n> in the old code.\n>\n> IOW, there is no change in behavior. The statement \"avoid wrap-around\n> issues\" in the subject line is not correct. The patch's only effect is\n> that it removes Warning C28159.\n>\n> What is really needed is that all quantities in the calculations are\n> promoted to ULONGLONG. Unless, of course, we agree that a timeout of\n> more than 49 days cannot happen ;)\n\nYep, correct on all counts. I'm in favor of changing the commit message to\nonly say that this patch removes Warning C28159.\n\nSteve\n"},{"id":"362284","messageId":"e8b7b173-eaa1-0fad-7e6a-771389872886@kdbg.org","threadId":"49727","inReplyTo":"CACbrTHctZejfDTjqWqVfPYdb=ssD253Cd2isr3BxWsL1AqsH2w@mail.gmail.com","subject":"Re: [PATCH 1/1] poll: use GetTickCount64() to avoid wrap-around issues","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2018-11-02T16:43:43Z","receivedAt":"2018-11-02T16:43:49Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 02.11.18 um 15:47 schrieb Steve Hoelzer:\n> On Thu, Nov 1, 2018 at 5:22 AM Johannes Sixt wrote:\n>>\n>> Am 31.10.18 um 22:11 schrieb Steve Hoelzer via GitGitGadget:\n>>> @@ -614,7 +618,7 @@ restart:\n>>>\n>>> if (!rc && orig_timeout && timeout != INFTIM)\n>>> {\n>>> - elapsed = GetTickCount() - start;\n>>> + elapsed = (DWORD)(GetTickCount64() - start);\n>>\n>> AFAICS, this subtraction in the old code is the correct way to take\n>> account of wrap-arounds in the tick count. The new code truncates the 64\n>> bit difference to 32 bits; the result is exactly identical to a\n>> difference computed from truncated 32 bit values, which is what we had\n>> in the old code.\n>>\n>> IOW, there is no change in behavior. The statement \"avoid wrap-around\n>> issues\" in the subject line is not correct. The patch's only effect is\n>> that it removes Warning C28159.\n>>\n>> What is really needed is that all quantities in the calculations are\n>> promoted to ULONGLONG. Unless, of course, we agree that a timeout of\n>> more than 49 days cannot happen ;)\n> \n> Yep, correct on all counts. I'm in favor of changing the commit message to\n> only say that this patch removes Warning C28159.\n\nHow about this fixup instead?\n\n---- 8< ----\nsquash! poll: use GetTickCount64() to avoid wrap-around issues\n\nThe value of timeout starts as an int value, and for this reason it\ncannot overflow unsigned long long aka ULONGLONG. The unsigned version\nof this initial value is available in orig_timeout. The difference\n(orig_timeout - elapsed) cannot wrap around because it is protected by\na conditional (as can be seen in the patch text). Hence, the ULONGLONG\ndifference can only have values that are smaller than the initial\ntimeout value and truncation to int cannot overflow.\n\nSigned-off-by: Johannes Sixt \n---\n compat/poll/poll.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/compat/poll/poll.c b/compat/poll/poll.c\nindex 4abbfcb6a4..4459408c7d 100644\n--- a/compat/poll/poll.c\n+++ b/compat/poll/poll.c\n@@ -452,7 +452,7 @@ poll (struct pollfd *pfd, nfds_t nfd, int timeout)\n static HANDLE hEvent;\n WSANETWORKEVENTS ev;\n HANDLE h, handle_array[FD_SETSIZE + 2];\n- DWORD ret, wait_timeout, nhandles, elapsed, orig_timeout = 0;\n+ DWORD ret, wait_timeout, nhandles, orig_timeout = 0;\n ULONGLONG start = 0;\n fd_set rfds, wfds, xfds;\n BOOL poll_again;\n@@ -618,8 +618,8 @@ poll (struct pollfd *pfd, nfds_t nfd, int timeout)\n \n if (!rc && orig_timeout && timeout != INFTIM)\n {\n- elapsed = (DWORD)(GetTickCount64() - start);\n- timeout = elapsed >= orig_timeout ? 0 : orig_timeout - elapsed;\n+ ULONGLONG elapsed = GetTickCount64() - start;\n+ timeout = elapsed >= orig_timeout ? 0 : (int)(orig_timeout - elapsed);\n }\n \n if (!rc && timeout)\n-- \n2.19.1.406.g1aa3f475f3\n"},{"id":"362288","messageId":"CACbrTHcvOQvHbSVbXhen6txksLxMqc4vub6qXVASsjzBt8BhUg@mail.gmail.com","threadId":"49727","inReplyTo":"e8b7b173-eaa1-0fad-7e6a-771389872886@kdbg.org","subject":"Re: [PATCH 1/1] poll: use GetTickCount64() to avoid wrap-around issues","fromName":"Steve Hoelzer","fromEmail":"shoelzer@gmail.com","sentAt":"2018-11-02T17:18:23Z","receivedAt":"2018-11-02T17:18:57Z","isPatch":true,"sender":{"key":"shoelzer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/4623209?v=4"},"body":"On Fri, Nov 2, 2018 at 11:43 AM Johannes Sixt wrote:\n>\n> Am 02.11.18 um 15:47 schrieb Steve Hoelzer:\n> > On Thu, Nov 1, 2018 at 5:22 AM Johannes Sixt wrote:\n> >>\n> >> Am 31.10.18 um 22:11 schrieb Steve Hoelzer via GitGitGadget:\n> >>> @@ -614,7 +618,7 @@ restart:\n> >>>\n> >>> if (!rc && orig_timeout && timeout != INFTIM)\n> >>> {\n> >>> - elapsed = GetTickCount() - start;\n> >>> + elapsed = (DWORD)(GetTickCount64() - start);\n> >>\n> >> AFAICS, this subtraction in the old code is the correct way to take\n> >> account of wrap-arounds in the tick count. The new code truncates the 64\n> >> bit difference to 32 bits; the result is exactly identical to a\n> >> difference computed from truncated 32 bit values, which is what we had\n> >> in the old code.\n> >>\n> >> IOW, there is no change in behavior. The statement \"avoid wrap-around\n> >> issues\" in the subject line is not correct. The patch's only effect is\n> >> that it removes Warning C28159.\n> >>\n> >> What is really needed is that all quantities in the calculations are\n> >> promoted to ULONGLONG. Unless, of course, we agree that a timeout of\n> >> more than 49 days cannot happen ;)\n> >\n> > Yep, correct on all counts. I'm in favor of changing the commit message to\n> > only say that this patch removes Warning C28159.\n>\n> How about this fixup instead?\n>\n> ---- 8< ----\n> squash! poll: use GetTickCount64() to avoid wrap-around issues\n>\n> The value of timeout starts as an int value, and for this reason it\n> cannot overflow unsigned long long aka ULONGLONG. The unsigned version\n> of this initial value is available in orig_timeout. The difference\n> (orig_timeout - elapsed) cannot wrap around because it is protected by\n> a conditional (as can be seen in the patch text). Hence, the ULONGLONG\n> difference can only have values that are smaller than the initial\n> timeout value and truncation to int cannot overflow.\n>\n> Signed-off-by: Johannes Sixt \n> ---\n> compat/poll/poll.c | 6 +++---\n> 1 file changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/compat/poll/poll.c b/compat/poll/poll.c\n> index 4abbfcb6a4..4459408c7d 100644\n> --- a/compat/poll/poll.c\n> +++ b/compat/poll/poll.c\n> @@ -452,7 +452,7 @@ poll (struct pollfd *pfd, nfds_t nfd, int timeout)\n> static HANDLE hEvent;\n> WSANETWORKEVENTS ev;\n> HANDLE h, handle_array[FD_SETSIZE + 2];\n> - DWORD ret, wait_timeout, nhandles, elapsed, orig_timeout = 0;\n> + DWORD ret, wait_timeout, nhandles, orig_timeout = 0;\n> ULONGLONG start = 0;\n> fd_set rfds, wfds, xfds;\n> BOOL poll_again;\n> @@ -618,8 +618,8 @@ poll (struct pollfd *pfd, nfds_t nfd, int timeout)\n>\n> if (!rc && orig_timeout && timeout != INFTIM)\n> {\n> - elapsed = (DWORD)(GetTickCount64() - start);\n> - timeout = elapsed >= orig_timeout ? 0 : orig_timeout - elapsed;\n> + ULONGLONG elapsed = GetTickCount64() - start;\n> + timeout = elapsed >= orig_timeout ? 0 : (int)(orig_timeout - elapsed);\n> }\n>\n> if (!rc && timeout)\n> --\n> 2.19.1.406.g1aa3f475f3\n\nI like it. This still removes the warning and avoids overflow issues.\n\nSteve\n"},{"id":"362325","messageId":"xmqqin1fngdx.fsf@gitster-ct.c.googlers.com","threadId":"49727","inReplyTo":"e8b7b173-eaa1-0fad-7e6a-771389872886@kdbg.org","subject":"Re: [PATCH 1/1] poll: use GetTickCount64() to avoid wrap-around issues","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-03T00:39:38Z","receivedAt":"2018-11-03T00:39:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n>> Yep, correct on all counts. I'm in favor of changing the commit message to\n>> only say that this patch removes Warning C28159.\n>\n> How about this fixup instead?\n\nIsn't that already in 'next'? I didn't check, though.\n\n"},{"id":"362326","messageId":"xmqqefc3nfwz.fsf@gitster-ct.c.googlers.com","threadId":"49727","inReplyTo":"xmqqin1fngdx.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] poll: use GetTickCount64() to avoid wrap-around issues","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-03T00:49:48Z","receivedAt":"2018-11-03T00:49:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Johannes Sixt writes:\n>\n>>> Yep, correct on all counts. I'm in favor of changing the commit message to\n>>> only say that this patch removes Warning C28159.\n>>\n>> How about this fixup instead?\n>\n> Isn't that already in 'next'? I didn't check, though.\n\nWell, it turnsout that I already prepared one but not pushed it out\nyet. I'll eject this topic and rebuild the integration branches,\nand wait until Dscho says something, to avoid having to redo the\nintegration cycle again.\n\nThanks.\n"},{"id":"362337","messageId":"CAPUEspgF0GjJPtMqmZjUmsEeaJpQQBBwOV9YOg8A6YBdwbdaFA@mail.gmail.com","threadId":"49727","inReplyTo":"e8b7b173-eaa1-0fad-7e6a-771389872886@kdbg.org","subject":"Re: [PATCH 1/1] poll: use GetTickCount64() to avoid wrap-around issues","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2018-11-03T08:14:56Z","receivedAt":"2018-11-03T08:15:13Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Fri, Nov 2, 2018 at 9:44 AM Johannes Sixt wrote:\n>\n> + timeout = elapsed >= orig_timeout ? 0 : (int)(orig_timeout - elapsed);\n\nnitpick: cast to DWORD instead of int\n\nCarlo\n"},{"id":"362360","messageId":"46aa1893-095b-9f0c-4989-e63ebaa88705@kdbg.org","threadId":"49727","inReplyTo":"CAPUEspgF0GjJPtMqmZjUmsEeaJpQQBBwOV9YOg8A6YBdwbdaFA@mail.gmail.com","subject":"Re: [PATCH 1/1] poll: use GetTickCount64() to avoid wrap-around issues","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2018-11-03T14:05:54Z","receivedAt":"2018-11-03T14:21:46Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 03.11.18 um 09:14 schrieb Carlo Arenas:\n> On Fri, Nov 2, 2018 at 9:44 AM Johannes Sixt wrote:\n>>\n>> + timeout = elapsed >= orig_timeout ? 0 : (int)(orig_timeout - elapsed);\n> \n> nitpick: cast to DWORD instead of int\n\nNo; timeout is of type int; after an explicit type cast we don't want to \nhave another implicit conversion.\n\n-- Hannes\n"},{"id":"362416","messageId":"xmqqefc0mnlh.fsf@gitster-ct.c.googlers.com","threadId":"49727","inReplyTo":"46aa1893-095b-9f0c-4989-e63ebaa88705@kdbg.org","subject":"Re: [PATCH 1/1] poll: use GetTickCount64() to avoid wrap-around issues","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-04T23:26:02Z","receivedAt":"2018-11-04T23:26:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> Am 03.11.18 um 09:14 schrieb Carlo Arenas:\n>> On Fri, Nov 2, 2018 at 9:44 AM Johannes Sixt wrote:\n>>>\n>>> + timeout = elapsed >= orig_timeout ? 0 : (int)(orig_timeout - elapsed);\n>>\n>> nitpick: cast to DWORD instead of int\n>\n> No; timeout is of type int; after an explicit type cast we don't want\n> to have another implicit conversion.\n>\n> -- Hannes\n\nOK, thanks. It seems that the relative silence after this message\nis a sign that the resulting patch after squashing is what everybody\nis happey with?\n\n-- >8 --\nFrom: Steve Hoelzer \nDate: Wed, 31 Oct 2018 14:11:36 -0700\nSubject: [PATCH] poll: use GetTickCount64() to avoid wrap-around issues\n\nThe value of timeout starts as an int value, and for this reason it\ncannot overflow unsigned long long aka ULONGLONG. The unsigned version\nof this initial value is available in orig_timeout. The difference\n(orig_timeout - elapsed) cannot wrap around because it is protected by\na conditional (as can be seen in the patch text). Hence, the ULONGLONG\ndifference can only have values that are smaller than the initial\ntimeout value and truncation to int cannot overflow.\n\nSigned-off-by: Johannes Sixt \nAcked-by: Steve Hoelzer \nSigned-off-by: Junio C Hamano \n---\n compat/poll/poll.c | 12 ++++++++----\n 1 file changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/compat/poll/poll.c b/compat/poll/poll.c\nindex ad5dcde439..4459408c7d 100644\n--- a/compat/poll/poll.c\n+++ b/compat/poll/poll.c\n@@ -18,6 +18,9 @@\n You should have received a copy of the GNU General Public License along\n with this program; if not, see . */\n \n+/* To bump the minimum Windows version to Windows Vista */\n+#include \"git-compat-util.h\"\n+\n /* Tell gcc not to warn about the (nfd < 0) tests, below. */\n #if (__GNUC__ == 4 && 3 <= __GNUC_MINOR__) || 4 < __GNUC__\n # pragma GCC diagnostic ignored \"-Wtype-limits\"\n@@ -449,7 +452,8 @@ poll (struct pollfd *pfd, nfds_t nfd, int timeout)\n static HANDLE hEvent;\n WSANETWORKEVENTS ev;\n HANDLE h, handle_array[FD_SETSIZE + 2];\n- DWORD ret, wait_timeout, nhandles, start = 0, elapsed, orig_timeout = 0;\n+ DWORD ret, wait_timeout, nhandles, orig_timeout = 0;\n+ ULONGLONG start = 0;\n fd_set rfds, wfds, xfds;\n BOOL poll_again;\n MSG msg;\n@@ -465,7 +469,7 @@ poll (struct pollfd *pfd, nfds_t nfd, int timeout)\n if (timeout != INFTIM)\n {\n orig_timeout = timeout;\n- start = GetTickCount();\n+ start = GetTickCount64();\n }\n \n if (!hEvent)\n@@ -614,8 +618,8 @@ poll (struct pollfd *pfd, nfds_t nfd, int timeout)\n \n if (!rc && orig_timeout && timeout != INFTIM)\n {\n- elapsed = GetTickCount() - start;\n- timeout = elapsed >= orig_timeout ? 0 : orig_timeout - elapsed;\n+ ULONGLONG elapsed = GetTickCount64() - start;\n+ timeout = elapsed >= orig_timeout ? 0 : (int)(orig_timeout - elapsed);\n }\n \n if (!rc && timeout)\n-- \n2.19.1-816-gcd69ec8cde\n\n"},{"id":"362417","messageId":"001101d47498$512b4bf0$f381e3d0$@nexbridge.com","threadId":"49727","inReplyTo":"xmqqefc0mnlh.fsf@gitster-ct.c.googlers.com","subject":"RE: [PATCH 1/1] poll: use GetTickCount64() to avoid wrap-around issues","fromName":"Randall S. Becker","fromEmail":"rsbecker@nexbridge.com","sentAt":"2018-11-04T23:44:18Z","receivedAt":"2018-11-04T23:45:01Z","isPatch":true,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On November 4, 2018 6:26 PM, Junio C Hamano, wrote:\n> Johannes Sixt writes:\n> \n> > Am 03.11.18 um 09:14 schrieb Carlo Arenas:\n> >> On Fri, Nov 2, 2018 at 9:44 AM Johannes Sixt wrote:\n> >>>\n> >>> + timeout = elapsed >= orig_timeout ? 0 : (int)(orig_timeout -\n> >>> + elapsed);\n> >>\n> >> nitpick: cast to DWORD instead of int\n> >\n> > No; timeout is of type int; after an explicit type cast we don't want\n> > to have another implicit conversion.\n> >\n> > -- Hannes\n> \n> OK, thanks. It seems that the relative silence after this message is a\nsign that\n> the resulting patch after squashing is what everybody is happey with?\n\nOn my platform (HPE NonStop), DWORD is being defined as unsigned int\n(32-bit) rather than unsigned long long (64 bit). The definition comes\nthrough the odbc/windows.h include, not the compiler or any core definition.\nIt's only a nano-quibble (if even that), because GetTickCount64 is not\ndefined on the platform anyway, so this is probably not a big deal.\n\nCheers,\nRandall\n\n\n"},{"id":"362447","messageId":"CAPig+cRdaXdZPpmMKBwZziMZarr2+wrdpnyHPkSYAkoBDuvLnw@mail.gmail.com","threadId":"49727","inReplyTo":"xmqqefc0mnlh.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] poll: use GetTickCount64() to avoid wrap-around issues","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-11-05T03:33:13Z","receivedAt":"2018-11-05T03:33:26Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Nov 4, 2018 at 6:26 PM Junio C Hamano wrote:\n> OK, thanks. It seems that the relative silence after this message\n> is a sign that the resulting patch after squashing is what everybody\n> is happey with?\n>\n> -- >8 --\n> From: Steve Hoelzer \n>\n> Signed-off-by: Johannes Sixt \n> Acked-by: Steve Hoelzer \n\nIt's not clear from this who the author is.\n"},{"id":"362450","messageId":"xmqq5zxcjho2.fsf@gitster-ct.c.googlers.com","threadId":"49727","inReplyTo":"CAPig+cRdaXdZPpmMKBwZziMZarr2+wrdpnyHPkSYAkoBDuvLnw@mail.gmail.com","subject":"Re: [PATCH 1/1] poll: use GetTickCount64() to avoid wrap-around issues","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-05T04:02:21Z","receivedAt":"2018-11-05T04:04:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> On Sun, Nov 4, 2018 at 6:26 PM Junio C Hamano wrote:\n>> OK, thanks. It seems that the relative silence after this message\n>> is a sign that the resulting patch after squashing is what everybody\n>> is happey with?\n>>\n>> -- >8 --\n>> From: Steve Hoelzer \n>>\n>> Signed-off-by: Johannes Sixt \n>> Acked-by: Steve Hoelzer \n>\n> It's not clear from this who the author is.\n\nRight. The latter should be s-o-b and the order swapped, and\nprobably say \"Steve wrote the original version, Johannes extended\nit\" in the text to match.\n\nTHanks.\n"},{"id":"362472","messageId":"8b22754b-89ec-ae04-c839-83810f93872f@kdbg.org","threadId":"49727","inReplyTo":"xmqqefc0mnlh.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] poll: use GetTickCount64() to avoid wrap-around issues","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2018-11-05T07:01:40Z","receivedAt":"2018-11-05T07:01:45Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 05.11.18 um 00:26 schrieb Junio C Hamano:\n> OK, thanks. It seems that the relative silence after this message\n> is a sign that the resulting patch after squashing is what everybody\n> is happey with?\n\nI'm not 100% happy. I'll resend a squashed patch, but it has to wait as \nI have to catch a train now.\n\nAppologies for the sub-optimal submission process.\n\n-- Hannes\n"},{"id":"362530","messageId":"3b9ed194-354d-627a-2c72-11173a3b9b45@kdbg.org","threadId":"49727","inReplyTo":"8b22754b-89ec-ae04-c839-83810f93872f@kdbg.org","subject":"Re: [PATCH 1/1] poll: use GetTickCount64() to avoid wrap-around issues","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2018-11-05T20:28:19Z","receivedAt":"2018-11-05T20:28:23Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 05.11.18 um 08:01 schrieb Johannes Sixt:\n> Am 05.11.18 um 00:26 schrieb Junio C Hamano:\n>> OK, thanks.  It seems that the relative silence after this message\n>> is a sign that the resulting patch after squashing is what everybody\n>> is happey with?\n> \n> I'm not 100% happy.\nI see the patch is already in next. Never mind. The patch text is fine, \nI just wanted to modify the commit message a bit.\n\nThanks,\n-- Hannes\n"},{"id":"362536","messageId":"nycvar.QRO.7.76.6.1811052304420.86@tvgsbejvaqbjf.bet","threadId":"49727","inReplyTo":"8b22754b-89ec-ae04-c839-83810f93872f@kdbg.org","subject":"Re: [PATCH 1/1] poll: use GetTickCount64() to avoid wrap-around issues","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-11-05T22:05:02Z","receivedAt":"2018-11-05T22:05:10Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Hannes,\n\nOn Mon, 5 Nov 2018, Johannes Sixt wrote:\n\n> Am 05.11.18 um 00:26 schrieb Junio C Hamano:\n> > OK, thanks. It seems that the relative silence after this message is\n> > a sign that the resulting patch after squashing is what everybody is\n> > happey with?\n> \n> I'm not 100% happy. I'll resend a squashed patch, but it has to wait as\n> I have to catch a train now.\n\nThank you for running with this.\n\nCiao,\nDscho\n"}]}