| Message ID | 20210324002015.338244-1-samuel.thibault@ens-lyon.org |
|---|---|
| State | New |
| Headers | show |
| Series | [hurd,commited] htl: Add missing fork.h | expand |
On 23/03/2021 21:20, Samuel Thibault wrote: > 2b47727c68b6 ("posix: Consolidate register-atfork") introduced a fork.h > header to declare the atfork unregister hook, but was missing adding it > for htl. > > This fixes tst-atfork2. Both lowlevellock.h and register-atfork.h are common headers, I think it would be better to just add them to generic fork.h header. > --- > sysdeps/htl/fork.h | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > create mode 100644 sysdeps/htl/fork.h > > diff --git a/sysdeps/htl/fork.h b/sysdeps/htl/fork.h > new file mode 100644 > index 0000000000..9d0d1d2b41 > --- /dev/null > +++ b/sysdeps/htl/fork.h > @@ -0,0 +1,20 @@ > +/* Copyright (C) 2002-2021 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + Contributed by Ulrich Drepper <drepper@redhat.com>, 2002. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <https://www.gnu.org/licenses/>. */ > + > +#include <lowlevellock.h> > +#include <register-atfork.h> >
Adhemerval Zanella, le mer. 24 mars 2021 09:29:44 -0300, a ecrit: > On 23/03/2021 21:20, Samuel Thibault wrote: > > 2b47727c68b6 ("posix: Consolidate register-atfork") introduced a fork.h > > header to declare the atfork unregister hook, but was missing adding it > > for htl. > > > > This fixes tst-atfork2. > > Both lowlevellock.h and register-atfork.h are common headers, > I think it would be better to just add them to generic fork.h header. Ok but perhaps there was a reason why you didn't do it in your commit? Samuel
On 24/03/2021 09:31, Samuel Thibault wrote: > Adhemerval Zanella, le mer. 24 mars 2021 09:29:44 -0300, a ecrit: >> On 23/03/2021 21:20, Samuel Thibault wrote: >>> 2b47727c68b6 ("posix: Consolidate register-atfork") introduced a fork.h >>> header to declare the atfork unregister hook, but was missing adding it >>> for htl. >>> >>> This fixes tst-atfork2. >> >> Both lowlevellock.h and register-atfork.h are common headers, >> I think it would be better to just add them to generic fork.h header. > > Ok but perhaps there was a reason why you didn't do it in your commit? I haven't touched the generic fork.h because I haven't see build issues on i686-gnu. Why exactly tst-atfork2 fails without this includes?
Adhemerval Zanella, le mer. 24 mars 2021 09:40:36 -0300, a ecrit:
> Why exactly tst-atfork2 fails without this includes?
It misses the atfork unregister hook called at module unload.
Samuel
On 24/03/2021 09:45, Samuel Thibault wrote: > Adhemerval Zanella, le mer. 24 mars 2021 09:40:36 -0300, a ecrit: >> Why exactly tst-atfork2 fails without this includes? > > It misses the atfork unregister hook called at module unload. Ok, but *why* exactly? The sysdeps/htl/pt-atfork.c already includes register-atfork.h after the fork.h inclusion. What the missing includes misses to define on hurd implementation?
Adhemerval Zanella, le mer. 24 mars 2021 09:56:47 -0300, a ecrit: > On 24/03/2021 09:45, Samuel Thibault wrote: > > Adhemerval Zanella, le mer. 24 mars 2021 09:40:36 -0300, a ecrit: > > > Why exactly tst-atfork2 fails without this includes? > > > > It misses the atfork unregister hook called at module unload. > > Ok, but *why* exactly? The sysdeps/htl/pt-atfork.c already > includes register-atfork.h after the fork.h inclusion. It's not pt-atfork.c which needs UNREGISTER_ATFORK, but stdlib/cxa_finalize, which gets it from fork.h Samuel
On 24/03/2021 10:00, Samuel Thibault wrote: > Adhemerval Zanella, le mer. 24 mars 2021 09:56:47 -0300, a ecrit: >> On 24/03/2021 09:45, Samuel Thibault wrote: >>> Adhemerval Zanella, le mer. 24 mars 2021 09:40:36 -0300, a ecrit: >>>> Why exactly tst-atfork2 fails without this includes? >>> >>> It misses the atfork unregister hook called at module unload. >> >> Ok, but *why* exactly? The sysdeps/htl/pt-atfork.c already >> includes register-atfork.h after the fork.h inclusion. > > It's not pt-atfork.c which needs UNREGISTER_ATFORK, but > stdlib/cxa_finalize, which gets it from fork.h Thanks, this is a good indication that is better to just remove the ifdef UNREGISTER_ATFORK from cxa_finalize (since now it is defined on generic implementation).
On 24/03/2021 10:04, Adhemerval Zanella wrote: > > > On 24/03/2021 10:00, Samuel Thibault wrote: >> Adhemerval Zanella, le mer. 24 mars 2021 09:56:47 -0300, a ecrit: >>> On 24/03/2021 09:45, Samuel Thibault wrote: >>>> Adhemerval Zanella, le mer. 24 mars 2021 09:40:36 -0300, a ecrit: >>>>> Why exactly tst-atfork2 fails without this includes? >>>> >>>> It misses the atfork unregister hook called at module unload. >>> >>> Ok, but *why* exactly? The sysdeps/htl/pt-atfork.c already >>> includes register-atfork.h after the fork.h inclusion. >> >> It's not pt-atfork.c which needs UNREGISTER_ATFORK, but >> stdlib/cxa_finalize, which gets it from fork.h > > Thanks, this is a good indication that is better to just remove > the ifdef UNREGISTER_ATFORK from cxa_finalize (since now it is > defined on generic implementation). And I think the fork.h header is not really required here, it is being used solely to define the NPTL specific __fork_generation, which Florian recent patch moves to libc.so. So I think we can move this declaration to another internal header and remove fork.h.
diff --git a/sysdeps/htl/fork.h b/sysdeps/htl/fork.h new file mode 100644 index 0000000000..9d0d1d2b41 --- /dev/null +++ b/sysdeps/htl/fork.h @@ -0,0 +1,20 @@ +/* Copyright (C) 2002-2021 Free Software Foundation, Inc. + This file is part of the GNU C Library. + Contributed by Ulrich Drepper <drepper@redhat.com>, 2002. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <https://www.gnu.org/licenses/>. */ + +#include <lowlevellock.h> +#include <register-atfork.h>
2b47727c68b6 ("posix: Consolidate register-atfork") introduced a fork.h header to declare the atfork unregister hook, but was missing adding it for htl. This fixes tst-atfork2. --- sysdeps/htl/fork.h | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 sysdeps/htl/fork.h