[review] hurd: Remove lingering references to the time function

Message ID gerrit.1572857958000.I37bc20f3449b9e358f32879ed231720c969965b4@gnutoolchain-gerrit.osci.io
State Superseded
Headers show
Series
  • [review] hurd: Remove lingering references to the time function
Related show

Commit Message

Simon Marchi (Code Review) Nov. 4, 2019, 8:59 a.m.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/505
......................................................................

hurd: Remove lingering references to the time function

They cause a check-localplt failure after commit f9a7554009cf38f39.

Fixes: f9a7554009cf38f390e74fcabc5b49f974f72382
Change-Id: I37bc20f3449b9e358f32879ed231720c969965b4
---
M sysdeps/mach/sleep.c
1 file changed, 2 insertions(+), 2 deletions(-)




-- 
Gerrit-Project: glibc
Gerrit-Branch: master
Gerrit-Change-Id: I37bc20f3449b9e358f32879ed231720c969965b4
Gerrit-Change-Number: 505
Gerrit-PatchSet: 1
Gerrit-Owner: Florian Weimer <fweimer@redhat.com>
Gerrit-MessageType: newchange

Comments

Simon Marchi (Code Review) Nov. 4, 2019, 11:26 a.m. | #1
Adhemerval Zanella has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/505
......................................................................


Patch Set 1: Code-Review+1

LGTM.


-- 
Gerrit-Project: glibc
Gerrit-Branch: master
Gerrit-Change-Id: I37bc20f3449b9e358f32879ed231720c969965b4
Gerrit-Change-Number: 505
Gerrit-PatchSet: 1
Gerrit-Owner: Florian Weimer <fweimer@redhat.com>
Gerrit-Reviewer: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Gerrit-Comment-Date: Mon, 04 Nov 2019 11:26:00 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Simon Marchi (Code Review) Nov. 4, 2019, 12:59 p.m. | #2
Adhemerval Zanella has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/505
......................................................................


Patch Set 1: Code-Review+2


-- 
Gerrit-Project: glibc
Gerrit-Branch: master
Gerrit-Change-Id: I37bc20f3449b9e358f32879ed231720c969965b4
Gerrit-Change-Number: 505
Gerrit-PatchSet: 1
Gerrit-Owner: Florian Weimer <fweimer@redhat.com>
Gerrit-Reviewer: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Gerrit-Comment-Date: Mon, 04 Nov 2019 12:59:56 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Samuel Thibault Nov. 4, 2019, 7:16 p.m. | #3
Florian Weimer (Code Review), le lun. 04 nov. 2019 03:59:18 -0500, a ecrit:
> Change URL: https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/505

> ......................................................................

> 

> hurd: Remove lingering references to the time function

> 

> They cause a check-localplt failure after commit f9a7554009cf38f39.

> 

> Fixes: f9a7554009cf38f390e74fcabc5b49f974f72382

> Change-Id: I37bc20f3449b9e358f32879ed231720c969965b4


Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>


Thanks!

> ---

> M sysdeps/mach/sleep.c

> 1 file changed, 2 insertions(+), 2 deletions(-)

> 

> 

> 

> diff --git a/sysdeps/mach/sleep.c b/sysdeps/mach/sleep.c

> index b30ec7b..267928e 100644

> --- a/sysdeps/mach/sleep.c

> +++ b/sysdeps/mach/sleep.c

> @@ -33,10 +33,10 @@

>  

>    recv = __mach_reply_port ();

>  

> -  before = time (NULL);

> +  before = time_now ();

>    (void) __mach_msg (NULL, MACH_RCV_MSG|MACH_RCV_TIMEOUT|MACH_RCV_INTERRUPT,

>  		     0, 0, recv, seconds * 1000, MACH_PORT_NULL);

> -  after = time (NULL);

> +  after = time_now ();

>    __mach_port_destroy (__mach_task_self (), recv);

>  

>    return seconds - (after - before);

> 

> -- 

> Gerrit-Project: glibc

> Gerrit-Branch: master

> Gerrit-Change-Id: I37bc20f3449b9e358f32879ed231720c969965b4

> Gerrit-Change-Number: 505

> Gerrit-PatchSet: 1

> Gerrit-Owner: Florian Weimer <fweimer@redhat.com>

> Gerrit-MessageType: newchange

>
Simon Marchi (Code Review) Nov. 7, 2019, 9:03 a.m. | #4
Florian Weimer has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/505
......................................................................


Patch Set 2: Code-Review+2


-- 
Gerrit-Project: glibc
Gerrit-Branch: master
Gerrit-Change-Id: I37bc20f3449b9e358f32879ed231720c969965b4
Gerrit-Change-Number: 505
Gerrit-PatchSet: 2
Gerrit-Owner: Florian Weimer <fweimer@redhat.com>
Gerrit-Reviewer: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Gerrit-Reviewer: Florian Weimer <fweimer@redhat.com>
Gerrit-Comment-Date: Thu, 07 Nov 2019 09:03:31 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Florian Weimer Nov. 7, 2019, 9:07 a.m. | #5
* Florian Weimer:

> Florian Weimer has posted comments on this change.

>

> Change URL: https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/505

> ......................................................................

>

>

> Patch Set 2: Code-Review+2


Not sure what was going on here.  The changeset did not show up as
merged in Gerrit after Gerrit saw it pushed.  Gerrit created a v2, and I
had to +2 it so that it got merged.

This did not happen with the other change,
<https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/506>, even though
pushing that created a v2 as well.

I added Reviewed-By: lines in both cases.

Simon, would you please have a look?  Is this an artifact of using
Gerrit in non-push mode?  Have you seen something similar with GDB?

Thanks,
Florian
Simon Marchi Nov. 7, 2019, 3:02 p.m. | #6
On 2019-11-07 4:07 a.m., Florian Weimer wrote:
> * Florian Weimer:

> 

>> Florian Weimer has posted comments on this change.

>>

>> Change URL: https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/505

>> ......................................................................

>>

>>

>> Patch Set 2: Code-Review+2

> 

> Not sure what was going on here.  The changeset did not show up as

> merged in Gerrit after Gerrit saw it pushed.  Gerrit created a v2, and I

> had to +2 it so that it got merged.

> 

> This did not happen with the other change,

> <https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/506>, even though

> pushing that created a v2 as well.

> 

> I added Reviewed-By: lines in both cases.

> 

> Simon, would you please have a look?  Is this an artifact of using

> Gerrit in non-push mode?  Have you seen something similar with GDB?


Yes, we have seen the same maybe 2-3 times (so it was more the exception
than the norm).  Gerrit marked the change as merged, but it did not
close it.  And as you said, doing some label change (like +2) seemed to
make Gerrit re-evaluate the state of the change correctly.

So it's a known bug.  I haven't really looked into it because it doesn't
happen often, so it's hard to reproduce.

Simon

Patch

diff --git a/sysdeps/mach/sleep.c b/sysdeps/mach/sleep.c
index b30ec7b..267928e 100644
--- a/sysdeps/mach/sleep.c
+++ b/sysdeps/mach/sleep.c
@@ -33,10 +33,10 @@ 
 
   recv = __mach_reply_port ();
 
-  before = time (NULL);
+  before = time_now ();
   (void) __mach_msg (NULL, MACH_RCV_MSG|MACH_RCV_TIMEOUT|MACH_RCV_INTERRUPT,
 		     0, 0, recv, seconds * 1000, MACH_PORT_NULL);
-  after = time (NULL);
+  after = time_now ();
   __mach_port_destroy (__mach_task_self (), recv);
 
   return seconds - (after - before);