diff mbox series

[1/1] poll: use GetTickCount64() to avoid wrap-around issues

Message ID 69bc5924f94b56f92d9653b3a64f721bd03f1956.1541020294.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series Make compat/poll safer on Windows | expand

Commit Message

Linus Arver via GitGitGadget Oct. 31, 2018, 9:11 p.m. UTC
From: Steve Hoelzer <shoelzer@gmail.com>

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@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 compat/poll/poll.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

Johannes Sixt Nov. 1, 2018, 10:21 a.m. UTC | #1
Am 31.10.18 um 22:11 schrieb Steve Hoelzer via GitGitGadget:
> From: Steve Hoelzer <shoelzer@gmail.com>
> 
>  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@gmail.com>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>   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 ;)

>         timeout = elapsed >= orig_timeout ? 0 : orig_timeout - elapsed;
>       }
>   

-- Hannes
Steve Hoelzer Nov. 2, 2018, 2:47 p.m. UTC | #2
On Thu, Nov 1, 2018 at 5:22 AM Johannes Sixt <j6t@kdbg.org> wrote:
>
> Am 31.10.18 um 22:11 schrieb Steve Hoelzer via GitGitGadget:
> > From: Steve Hoelzer <shoelzer@gmail.com>
> >
> >  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@gmail.com>
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >   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
Johannes Sixt Nov. 2, 2018, 4:43 p.m. UTC | #3
Am 02.11.18 um 15:47 schrieb Steve Hoelzer:
> On Thu, Nov 1, 2018 at 5:22 AM Johannes Sixt <j6t@kdbg.org> wrote:
>>
>> Am 31.10.18 um 22:11 schrieb Steve Hoelzer via GitGitGadget:
>>> @@ -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.

How about this fixup instead?

---- 8< ----
squash! poll: use GetTickCount64() to avoid wrap-around issues

The value of timeout starts as an int value, and for this reason it
cannot overflow unsigned long long aka ULONGLONG. The unsigned version
of this initial value is available in orig_timeout. The difference
(orig_timeout - elapsed) cannot wrap around because it is protected by
a conditional (as can be seen in the patch text). Hence, the ULONGLONG
difference can only have values that are smaller than the initial
timeout value and truncation to int cannot overflow.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 compat/poll/poll.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/compat/poll/poll.c b/compat/poll/poll.c
index 4abbfcb6a4..4459408c7d 100644
--- a/compat/poll/poll.c
+++ b/compat/poll/poll.c
@@ -452,7 +452,7 @@ 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, elapsed, orig_timeout = 0;
+  DWORD ret, wait_timeout, nhandles, orig_timeout = 0;
   ULONGLONG start = 0;
   fd_set rfds, wfds, xfds;
   BOOL poll_again;
@@ -618,8 +618,8 @@ poll (struct pollfd *pfd, nfds_t nfd, int timeout)
 
   if (!rc && orig_timeout && timeout != INFTIM)
     {
-      elapsed = (DWORD)(GetTickCount64() - start);
-      timeout = elapsed >= orig_timeout ? 0 : orig_timeout - elapsed;
+      ULONGLONG elapsed = GetTickCount64() - start;
+      timeout = elapsed >= orig_timeout ? 0 : (int)(orig_timeout - elapsed);
     }
 
   if (!rc && timeout)
Steve Hoelzer Nov. 2, 2018, 5:18 p.m. UTC | #4
On Fri, Nov 2, 2018 at 11:43 AM Johannes Sixt <j6t@kdbg.org> wrote:
>
> Am 02.11.18 um 15:47 schrieb Steve Hoelzer:
> > On Thu, Nov 1, 2018 at 5:22 AM Johannes Sixt <j6t@kdbg.org> wrote:
> >>
> >> Am 31.10.18 um 22:11 schrieb Steve Hoelzer via GitGitGadget:
> >>> @@ -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.
>
> How about this fixup instead?
>
> ---- 8< ----
> squash! poll: use GetTickCount64() to avoid wrap-around issues
>
> The value of timeout starts as an int value, and for this reason it
> cannot overflow unsigned long long aka ULONGLONG. The unsigned version
> of this initial value is available in orig_timeout. The difference
> (orig_timeout - elapsed) cannot wrap around because it is protected by
> a conditional (as can be seen in the patch text). Hence, the ULONGLONG
> difference can only have values that are smaller than the initial
> timeout value and truncation to int cannot overflow.
>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
>  compat/poll/poll.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/compat/poll/poll.c b/compat/poll/poll.c
> index 4abbfcb6a4..4459408c7d 100644
> --- a/compat/poll/poll.c
> +++ b/compat/poll/poll.c
> @@ -452,7 +452,7 @@ 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, elapsed, orig_timeout = 0;
> +  DWORD ret, wait_timeout, nhandles, orig_timeout = 0;
>    ULONGLONG start = 0;
>    fd_set rfds, wfds, xfds;
>    BOOL poll_again;
> @@ -618,8 +618,8 @@ poll (struct pollfd *pfd, nfds_t nfd, int timeout)
>
>    if (!rc && orig_timeout && timeout != INFTIM)
>      {
> -      elapsed = (DWORD)(GetTickCount64() - start);
> -      timeout = elapsed >= orig_timeout ? 0 : orig_timeout - elapsed;
> +      ULONGLONG elapsed = GetTickCount64() - start;
> +      timeout = elapsed >= orig_timeout ? 0 : (int)(orig_timeout - elapsed);
>      }
>
>    if (!rc && timeout)
> --
> 2.19.1.406.g1aa3f475f3

I like it. This still removes the warning and avoids overflow issues.

Steve
Junio C Hamano Nov. 3, 2018, 12:39 a.m. UTC | #5
Johannes Sixt <j6t@kdbg.org> writes:

>> Yep, correct on all counts. I'm in favor of changing the commit message to
>> only say that this patch removes Warning C28159.
>
> How about this fixup instead?

Isn't that already in 'next'?  I didn't check, though.
Junio C Hamano Nov. 3, 2018, 12:49 a.m. UTC | #6
Junio C Hamano <gitster@pobox.com> writes:

> Johannes Sixt <j6t@kdbg.org> writes:
>
>>> Yep, correct on all counts. I'm in favor of changing the commit message to
>>> only say that this patch removes Warning C28159.
>>
>> How about this fixup instead?
>
> Isn't that already in 'next'?  I didn't check, though.

Well, it turnsout that I already prepared one but not pushed it out
yet.  I'll eject this topic and rebuild the integration branches,
and wait until Dscho says something, to avoid having to redo the
integration cycle again.

Thanks.
Carlo Marcelo Arenas Belón Nov. 3, 2018, 8:14 a.m. UTC | #7
On Fri, Nov 2, 2018 at 9:44 AM Johannes Sixt <j6t@kdbg.org> wrote:
>
> +      timeout = elapsed >= orig_timeout ? 0 : (int)(orig_timeout - elapsed);

nitpick: cast to DWORD instead of int

Carlo
Johannes Sixt Nov. 3, 2018, 2:05 p.m. UTC | #8
Am 03.11.18 um 09:14 schrieb Carlo Arenas:
> On Fri, Nov 2, 2018 at 9:44 AM Johannes Sixt <j6t@kdbg.org> wrote:
>>
>> +      timeout = elapsed >= orig_timeout ? 0 : (int)(orig_timeout - elapsed);
> 
> nitpick: cast to DWORD instead of int

No; timeout is of type int; after an explicit type cast we don't want to 
have another implicit conversion.

-- Hannes
Junio C Hamano Nov. 4, 2018, 11:26 p.m. UTC | #9
Johannes Sixt <j6t@kdbg.org> writes:

> Am 03.11.18 um 09:14 schrieb Carlo Arenas:
>> On Fri, Nov 2, 2018 at 9:44 AM Johannes Sixt <j6t@kdbg.org> wrote:
>>>
>>> +      timeout = elapsed >= orig_timeout ? 0 : (int)(orig_timeout - elapsed);
>>
>> nitpick: cast to DWORD instead of int
>
> No; timeout is of type int; after an explicit type cast we don't want
> to have another implicit conversion.
>
> -- Hannes

OK, thanks.  It seems that the relative silence after this message
is a sign that the resulting patch after squashing is what everybody
is happey with?

-- >8 --
From: Steve Hoelzer <shoelzer@gmail.com>
Date: Wed, 31 Oct 2018 14:11:36 -0700
Subject: [PATCH] poll: use GetTickCount64() to avoid wrap-around issues

The value of timeout starts as an int value, and for this reason it
cannot overflow unsigned long long aka ULONGLONG. The unsigned version
of this initial value is available in orig_timeout. The difference
(orig_timeout - elapsed) cannot wrap around because it is protected by
a conditional (as can be seen in the patch text). Hence, the ULONGLONG
difference can only have values that are smaller than the initial
timeout value and truncation to int cannot overflow.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Acked-by: Steve Hoelzer <shoelzer@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 compat/poll/poll.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/compat/poll/poll.c b/compat/poll/poll.c
index ad5dcde439..4459408c7d 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, 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,8 +618,8 @@ poll (struct pollfd *pfd, nfds_t nfd, int timeout)
 
   if (!rc && orig_timeout && timeout != INFTIM)
     {
-      elapsed = GetTickCount() - start;
-      timeout = elapsed >= orig_timeout ? 0 : orig_timeout - elapsed;
+      ULONGLONG elapsed = GetTickCount64() - start;
+      timeout = elapsed >= orig_timeout ? 0 : (int)(orig_timeout - elapsed);
     }
 
   if (!rc && timeout)
Randall S. Becker Nov. 4, 2018, 11:44 p.m. UTC | #10
On November 4, 2018 6:26 PM, Junio C Hamano, wrote:
> Johannes Sixt <j6t@kdbg.org> writes:
> 
> > Am 03.11.18 um 09:14 schrieb Carlo Arenas:
> >> On Fri, Nov 2, 2018 at 9:44 AM Johannes Sixt <j6t@kdbg.org> wrote:
> >>>
> >>> +      timeout = elapsed >= orig_timeout ? 0 : (int)(orig_timeout -
> >>> + elapsed);
> >>
> >> nitpick: cast to DWORD instead of int
> >
> > No; timeout is of type int; after an explicit type cast we don't want
> > to have another implicit conversion.
> >
> > -- Hannes
> 
> OK, thanks.  It seems that the relative silence after this message is a
sign that
> the resulting patch after squashing is what everybody is happey with?

On my platform (HPE NonStop), DWORD is being defined as unsigned int
(32-bit) rather than unsigned long long (64 bit). The definition comes
through the odbc/windows.h include, not the compiler or any core definition.
It's only a nano-quibble (if even that), because GetTickCount64 is not
defined on the platform anyway, so this is probably not a big deal.

Cheers,
Randall
Eric Sunshine Nov. 5, 2018, 3:33 a.m. UTC | #11
On Sun, Nov 4, 2018 at 6:26 PM Junio C Hamano <gitster@pobox.com> wrote:
> OK, thanks.  It seems that the relative silence after this message
> is a sign that the resulting patch after squashing is what everybody
> is happey with?
>
> -- >8 --
> From: Steve Hoelzer <shoelzer@gmail.com>
>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> Acked-by: Steve Hoelzer <shoelzer@gmail.com>

It's not clear from this who the author is.
Junio C Hamano Nov. 5, 2018, 4:02 a.m. UTC | #12
Eric Sunshine <sunshine@sunshineco.com> writes:

> On Sun, Nov 4, 2018 at 6:26 PM Junio C Hamano <gitster@pobox.com> wrote:
>> OK, thanks.  It seems that the relative silence after this message
>> is a sign that the resulting patch after squashing is what everybody
>> is happey with?
>>
>> -- >8 --
>> From: Steve Hoelzer <shoelzer@gmail.com>
>>
>> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
>> Acked-by: Steve Hoelzer <shoelzer@gmail.com>
>
> It's not clear from this who the author is.

Right.  The latter should be s-o-b and the order swapped, and
probably say "Steve wrote the original version, Johannes extended
it" in the text to match.

THanks.
Johannes Sixt Nov. 5, 2018, 7:01 a.m. UTC | #13
Am 05.11.18 um 00:26 schrieb Junio C Hamano:
> OK, thanks.  It seems that the relative silence after this message
> is a sign that the resulting patch after squashing is what everybody
> is happey with?

I'm not 100% happy. I'll resend a squashed patch, but it has to wait as 
I have to catch a train now.

Appologies for the sub-optimal submission process.

-- Hannes
Johannes Sixt Nov. 5, 2018, 8:28 p.m. UTC | #14
Am 05.11.18 um 08:01 schrieb Johannes Sixt:
> Am 05.11.18 um 00:26 schrieb Junio C Hamano:
>> OK, thanks.  It seems that the relative silence after this message
>> is a sign that the resulting patch after squashing is what everybody
>> is happey with?
> 
> I'm not 100% happy.
I see the patch is already in next. Never mind. The patch text is fine, 
I just wanted to modify the commit message a bit.

Thanks,
-- Hannes
Johannes Schindelin Nov. 5, 2018, 10:05 p.m. UTC | #15
Hi Hannes,

On Mon, 5 Nov 2018, Johannes Sixt wrote:

> Am 05.11.18 um 00:26 schrieb Junio C Hamano:
> > OK, thanks.  It seems that the relative silence after this message is
> > a sign that the resulting patch after squashing is what everybody is
> > happey with?
> 
> I'm not 100% happy. I'll resend a squashed patch, but it has to wait as
> I have to catch a train now.

Thank you for running with this.

Ciao,
Dscho
diff mbox series

Patch

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);
       timeout = elapsed >= orig_timeout ? 0 : orig_timeout - elapsed;
     }