[01/23] hurd: Add some missing includes
Checks
Context |
Check |
Description |
redhat-pt-bot/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
redhat-pt-bot/TryBot-still_applies |
warning
|
Patch no longer applies to master
|
Commit Message
Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
hurd/hurdsig.c | 2 +-
mach/lowlevellock.h | 1 +
sysdeps/hurd/include/hurd.h | 1 +
sysdeps/hurd/include/hurd/signal.h | 1 +
sysdeps/mach/hurd/mig-reply.c | 1 +
5 files changed, 5 insertions(+), 1 deletion(-)
Comments
Sergey Bugaev, le mer. 03 janv. 2024 20:14:34 +0300, a ecrit:
> diff --git a/sysdeps/hurd/include/hurd.h b/sysdeps/hurd/include/hurd.h
> index 568092d6..189fd44e 100644
> --- a/sysdeps/hurd/include/hurd.h
> +++ b/sysdeps/hurd/include/hurd.h
> @@ -1,4 +1,5 @@
> #ifndef _HURD_H
> +#include <tls.h>
> #include_next <hurd.h>
>
> void _hurd_libc_proc_init (char **argv);
> diff --git a/sysdeps/hurd/include/hurd/signal.h b/sysdeps/hurd/include/hurd/signal.h
> index 1dc8a1f3..9b1bf3df 100644
> --- a/sysdeps/hurd/include/hurd/signal.h
> +++ b/sysdeps/hurd/include/hurd/signal.h
> @@ -6,6 +6,7 @@ extern struct hurd_sigstate *_hurd_self_sigstate (void) __attribute__ ((__const_
> libc_hidden_proto (_hurd_self_sigstate)
> #endif
>
> +#include <tls.h>
> #include_next <hurd/signal.h>
>
> #ifndef _ISOMAC
Why?
These are breaking hurd/check-installed-headers-c
Samuel
Applied the rest, thanks!
Sergey Bugaev, le mer. 03 janv. 2024 20:14:34 +0300, a ecrit:
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
> hurd/hurdsig.c | 2 +-
> mach/lowlevellock.h | 1 +
> sysdeps/hurd/include/hurd.h | 1 +
> sysdeps/hurd/include/hurd/signal.h | 1 +
> sysdeps/mach/hurd/mig-reply.c | 1 +
> 5 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/hurd/hurdsig.c b/hurd/hurdsig.c
> index 3deb94f9..fe788193 100644
> --- a/hurd/hurdsig.c
> +++ b/hurd/hurdsig.c
> @@ -15,6 +15,7 @@
> License along with the GNU C Library; if not, see
> <https://www.gnu.org/licenses/>. */
>
> +#include <assert.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> @@ -260,7 +261,6 @@ _hurd_sigstate_actions (struct hurd_sigstate *ss)
> #include <hurd/msg_server.h>
> #include <hurd/msg_reply.h> /* For __msg_sig_post_reply. */
> #include <hurd/interrupt.h>
> -#include <assert.h>
> #include <unistd.h>
>
>
> diff --git a/mach/lowlevellock.h b/mach/lowlevellock.h
> index 129a7bd9..c5bb0879 100644
> --- a/mach/lowlevellock.h
> +++ b/mach/lowlevellock.h
> @@ -19,6 +19,7 @@
> #ifndef _MACH_LOWLEVELLOCK_H
> #define _MACH_LOWLEVELLOCK_H 1
>
> +#include <mach.h>
> #include <mach/gnumach.h>
> #include <atomic.h>
>
> diff --git a/sysdeps/hurd/include/hurd.h b/sysdeps/hurd/include/hurd.h
> index 568092d6..189fd44e 100644
> --- a/sysdeps/hurd/include/hurd.h
> +++ b/sysdeps/hurd/include/hurd.h
> @@ -1,4 +1,5 @@
> #ifndef _HURD_H
> +#include <tls.h>
> #include_next <hurd.h>
>
> void _hurd_libc_proc_init (char **argv);
> diff --git a/sysdeps/hurd/include/hurd/signal.h b/sysdeps/hurd/include/hurd/signal.h
> index 1dc8a1f3..9b1bf3df 100644
> --- a/sysdeps/hurd/include/hurd/signal.h
> +++ b/sysdeps/hurd/include/hurd/signal.h
> @@ -6,6 +6,7 @@ extern struct hurd_sigstate *_hurd_self_sigstate (void) __attribute__ ((__const_
> libc_hidden_proto (_hurd_self_sigstate)
> #endif
>
> +#include <tls.h>
> #include_next <hurd/signal.h>
>
> #ifndef _ISOMAC
> diff --git a/sysdeps/mach/hurd/mig-reply.c b/sysdeps/mach/hurd/mig-reply.c
> index 57bcd808..85c3af13 100644
> --- a/sysdeps/mach/hurd/mig-reply.c
> +++ b/sysdeps/mach/hurd/mig-reply.c
> @@ -15,6 +15,7 @@
> License along with the GNU C Library; if not, see
> <https://www.gnu.org/licenses/>. */
>
> +#include <assert.h>
> #include <mach.h>
> #include <mach/mig_support.h>
> #include <tls.h>
> --
> 2.43.0
>
>
On Wed, Jan 3, 2024 at 11:43 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> Sergey Bugaev, le mer. 03 janv. 2024 20:14:34 +0300, a ecrit:
> > diff --git a/sysdeps/hurd/include/hurd.h b/sysdeps/hurd/include/hurd.h
> > index 568092d6..189fd44e 100644
> > --- a/sysdeps/hurd/include/hurd.h
> > +++ b/sysdeps/hurd/include/hurd.h
> > @@ -1,4 +1,5 @@
> > #ifndef _HURD_H
> > +#include <tls.h>
> > #include_next <hurd.h>
> >
> > void _hurd_libc_proc_init (char **argv);
> > diff --git a/sysdeps/hurd/include/hurd/signal.h b/sysdeps/hurd/include/hurd/signal.h
> > index 1dc8a1f3..9b1bf3df 100644
> > --- a/sysdeps/hurd/include/hurd/signal.h
> > +++ b/sysdeps/hurd/include/hurd/signal.h
> > @@ -6,6 +6,7 @@ extern struct hurd_sigstate *_hurd_self_sigstate (void) __attribute__ ((__const_
> > libc_hidden_proto (_hurd_self_sigstate)
> > #endif
> >
> > +#include <tls.h>
> > #include_next <hurd/signal.h>
> >
> > #ifndef _ISOMAC
>
> Why?
Because hurd/hurd/signal.h is using tls.h macros (THREAD_GETMEM /
THREAD_SETMEM / THREAD_SELF), guarded under "defined _LIBC", and
sysdeps/hurd/include/hurd/signal.h being the internal version of that
header seemed to be the appropriate place to add the missing #include
<tls.h>. Otherwise, I get this:
In file included from ../sysdeps/hurd/include/hurd/signal.h:9,
from siginfo.c:18:
../hurd/hurd/signal.h: In function ‘_hurd_self_sigstate’:
../hurd/hurd/signal.h:169:30: error: implicit declaration of function
‘THREAD_GETMEM’ [-Wimplicit-function-declaration]
169 | struct hurd_sigstate *ss = THREAD_GETMEM (THREAD_SELF,
_hurd_sigstate);
and so on. This must have happened to work on both x86 architectures
due to some other header implicitly pulling <tls.h> in, but we should
not rely on that.
Perhaps a better solution would be to move the inline versions of
_hurd_self_sigstate and _hurd_critical_section_lock/unlock to the
internal header. Is there any reason why they have to be in the public
one?
> These are breaking hurd/check-installed-headers-c
Indeed, thanks for pointing that out. But the error I seem to get:
:: hurd.h
::::
In file included from ../sysdeps/unix/i386/sysdep.h:18,
from ../sysdeps/mach/x86/sysdep.h:47,
from ../sysdeps/mach/hurd/tls.h:27,
from ../sysdeps/mach/hurd/i386/tls.h:24,
from ../sysdeps/hurd/include/hurd.h:2,
from /tmp/cih_test_z99fCI.c:10:
../sysdeps/unix/sysdep.h:111:5: error: "IS_IN" is not defined,
evaluates to 0 [-Werror=undef]
111 | #if IS_IN (rtld)
| ^~~~~
../sysdeps/unix/sysdep.h:111:11: error: missing binary operator before token "("
111 | #if IS_IN (rtld)
| ^
../sysdeps/mach/hurd/i386/tls.h:123:32: error: missing binary operator
before token "("
123 | #if !defined (SHARED) || IS_IN (rtld)
| ^
../sysdeps/mach/hurd/i386/tls.h: In function ‘_hurd_tls_fork’:
../sysdeps/mach/hurd/i386/tls.h:379:3: error: unknown type name ‘error_t’
379 | error_t err;
| ^~~~~~~
...makes no sense. This is testing installed headers, isn't it? — then
how come sysdeps/hurd/include/hurd.h is what gets found for <hurd.h>?
I'm rather sure the installed <hurd.h> is a different file. So it
would look like the test setup is broken, and this patch just exposes
that.
Sergey
On Thu, Jan 4, 2024 at 12:08 AM Sergey Bugaev <bugaevc@gmail.com> wrote:
> This is testing installed headers, isn't it? — then
> how come sysdeps/hurd/include/hurd.h is what gets found for <hurd.h>?
> I'm rather sure the installed <hurd.h> is a different file. So it
> would look like the test setup is broken, and this patch just exposes
> that.
Should the include <tls.h> be guarded by ifndef _ISOMAC perhaps? And
if we indeed move the inline functions to the internal hurd.h, they'd
also be guarded by ifndef _ISOMAC?
But really, why doesn't the test filter out the internal include directories?
Sergey
Sergey Bugaev, le jeu. 04 janv. 2024 00:24:30 +0300, a ecrit:
> On Thu, Jan 4, 2024 at 12:08 AM Sergey Bugaev <bugaevc@gmail.com> wrote:
> > This is testing installed headers, isn't it? — then
> > how come sysdeps/hurd/include/hurd.h is what gets found for <hurd.h>?
> > I'm rather sure the installed <hurd.h> is a different file. So it
> > would look like the test setup is broken, and this patch just exposes
> > that.
>
> Should the include <tls.h> be guarded by ifndef _ISOMAC perhaps? And
> if we indeed move the inline functions to the internal hurd.h, they'd
> also be guarded by ifndef _ISOMAC?
Yes.
> But really, why doesn't the test filter out the internal include directories?
I'll leave that to the people who would know examples of needing
internal definitions.
Samuel
@@ -15,6 +15,7 @@
License along with the GNU C Library; if not, see
<https://www.gnu.org/licenses/>. */
+#include <assert.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
@@ -260,7 +261,6 @@ _hurd_sigstate_actions (struct hurd_sigstate *ss)
#include <hurd/msg_server.h>
#include <hurd/msg_reply.h> /* For __msg_sig_post_reply. */
#include <hurd/interrupt.h>
-#include <assert.h>
#include <unistd.h>
@@ -19,6 +19,7 @@
#ifndef _MACH_LOWLEVELLOCK_H
#define _MACH_LOWLEVELLOCK_H 1
+#include <mach.h>
#include <mach/gnumach.h>
#include <atomic.h>
@@ -1,4 +1,5 @@
#ifndef _HURD_H
+#include <tls.h>
#include_next <hurd.h>
void _hurd_libc_proc_init (char **argv);
@@ -6,6 +6,7 @@ extern struct hurd_sigstate *_hurd_self_sigstate (void) __attribute__ ((__const_
libc_hidden_proto (_hurd_self_sigstate)
#endif
+#include <tls.h>
#include_next <hurd/signal.h>
#ifndef _ISOMAC
@@ -15,6 +15,7 @@
License along with the GNU C Library; if not, see
<https://www.gnu.org/licenses/>. */
+#include <assert.h>
#include <mach.h>
#include <mach/mig_support.h>
#include <tls.h>