Update hurd/hurdselect.c to be more portable.

Message ID ZFa+roan3ioo0ONM@jupiter.tail36e24.ts.net
State Committed, archived
Headers
Series Update hurd/hurdselect.c to be more portable. |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit success Build for i686

Commit Message

Flavio Cruz May 6, 2023, 8:55 p.m. UTC
  Summary of changes:
- Use BAD_TYPECHECK to perform type checking in a cleaner way.
  BAD_TYPECHECK is moved into sysdeps/mach/rpc.h to avoid duplication.
- Remove assertions for mach_msg_type_t since those won't work for
  x86_64.
- Update message structs to use mach_msg_type_t directly.
- Use designated initializers.
---
 hurd/hurdselect.c         | 30 ++++++++++++++----------------
 hurd/intr-msg.c           |  6 +-----
 sysdeps/mach/hurd/ioctl.c |  6 +-----
 sysdeps/mach/mach_rpc.h   | 31 +++++++++++++++++++++++++++++++
 4 files changed, 47 insertions(+), 26 deletions(-)
 create mode 100644 sysdeps/mach/mach_rpc.h
  

Comments

Samuel Thibault May 6, 2023, 9:11 p.m. UTC | #1
Applied, thanks!

Flavio Cruz, le sam. 06 mai 2023 16:55:10 -0400, a ecrit:
> Summary of changes:
> - Use BAD_TYPECHECK to perform type checking in a cleaner way.
>   BAD_TYPECHECK is moved into sysdeps/mach/rpc.h to avoid duplication.
> - Remove assertions for mach_msg_type_t since those won't work for
>   x86_64.
> - Update message structs to use mach_msg_type_t directly.
> - Use designated initializers.
> ---
>  hurd/hurdselect.c         | 30 ++++++++++++++----------------
>  hurd/intr-msg.c           |  6 +-----
>  sysdeps/mach/hurd/ioctl.c |  6 +-----
>  sysdeps/mach/mach_rpc.h   | 31 +++++++++++++++++++++++++++++++
>  4 files changed, 47 insertions(+), 26 deletions(-)
>  create mode 100644 sysdeps/mach/mach_rpc.h
> 
> diff --git a/hurd/hurdselect.c b/hurd/hurdselect.c
> index 08fcc92170..9630cae474 100644
> --- a/hurd/hurdselect.c
> +++ b/hurd/hurdselect.c
> @@ -22,6 +22,7 @@
>  #include <hurd.h>
>  #include <hurd/fd.h>
>  #include <hurd/io_request.h>
> +#include <mach_rpc.h>
>  #include <stdlib.h>
>  #include <string.h>
>  #include <assert.h>
> @@ -69,14 +70,6 @@ _hurd_select (int nfds,
>    sigset_t oset;
>    struct hurd_sigstate *ss = NULL;
>  
> -  union typeword		/* Use this to avoid unkosher casts.  */
> -    {
> -      mach_msg_type_t type;
> -      uint32_t word;
> -    };
> -  assert (sizeof (union typeword) == sizeof (mach_msg_type_t));
> -  assert (sizeof (uint32_t) == sizeof (mach_msg_type_t));
> -
>    if (nfds < 0 || (pollfds == NULL && nfds > FD_SETSIZE))
>      {
>        errno = EINVAL;
> @@ -404,15 +397,15 @@ _hurd_select (int nfds,
>  	  struct
>  	    {
>  	      mach_msg_header_t head;
> -	      union typeword err_type;
> +	      mach_msg_type_t err_type;
>  	      error_t err;
>  	    } error;
>  	  struct
>  	    {
>  	      mach_msg_header_t head;
> -	      union typeword err_type;
> +	      mach_msg_type_t err_type;
>  	      error_t err;
> -	      union typeword result_type;
> +	      mach_msg_type_t result_type;
>  	      int result;
>  	    } success;
>  #endif
> @@ -443,9 +436,14 @@ _hurd_select (int nfds,
>  
>  	  /* We got a message.  Decode it.  */
>  #ifdef MACH_MSG_TYPE_BIT
> -	  const union typeword inttype =
> -	  { type:
> -	    { MACH_MSG_TYPE_INTEGER_T, sizeof (integer_t) * 8, 1, 1, 0, 0 }
> +	  static const mach_msg_type_t inttype = {
> +	    .msgt_name = MACH_MSG_TYPE_INTEGER_T,
> +	    .msgt_size = sizeof (integer_t) * 8,
> +	    .msgt_number = 1,
> +	    .msgt_inline = TRUE,
> +	    .msgt_longform = FALSE,
> +	    .msgt_deallocate = FALSE,
> +	    .msgt_unused = 0
>  	  };
>  #endif
>  
> @@ -462,7 +460,7 @@ _hurd_select (int nfds,
>  	      && msg.head.msgh_size >= sizeof msg.error
>  	      && !(msg.head.msgh_bits & MACH_MSGH_BITS_COMPLEX)
>  #ifdef MACH_MSG_TYPE_BIT
> -	      && msg.error.err_type.word == inttype.word
> +	      && !BAD_TYPECHECK (&msg.error.err_type, &inttype)
>  #endif
>  	      )
>  	    {
> @@ -480,7 +478,7 @@ _hurd_select (int nfds,
>  		 occurred.  */
>  	      if (msg.error.err
>  #ifdef MACH_MSG_TYPE_BIT
> -		  || msg.success.result_type.word != inttype.word
> +		  || BAD_TYPECHECK (&msg.success.result_type, &inttype)
>  #endif
>  		  || msg.head.msgh_size != sizeof msg.success)
>  		{
> diff --git a/hurd/intr-msg.c b/hurd/intr-msg.c
> index b535397bfb..bc1f43d383 100644
> --- a/hurd/intr-msg.c
> +++ b/hurd/intr-msg.c
> @@ -17,6 +17,7 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #include <mach.h>
> +#include <mach_rpc.h>
>  #include <mach/mig_errors.h>
>  #include <mach/mig_support.h>
>  #include <hurd/signal.h>
> @@ -28,11 +29,6 @@
>  # define mig_reply_header_t	mig_reply_error_t
>  #endif
>  
> -/* Macro used by MIG to cleanly check the type.  */
> -#define BAD_TYPECHECK(type, check) __glibc_unlikely (({	\
> -  union { mach_msg_type_t t; uint32_t w; } _t, _c;	\
> -  _t.t = *(type); _c.t = *(check);_t.w != _c.w; }))
> -
>  error_t
>  _hurd_intr_rpc_mach_msg (mach_msg_header_t *msg,
>  			 mach_msg_option_t option,
> diff --git a/sysdeps/mach/hurd/ioctl.c b/sysdeps/mach/hurd/ioctl.c
> index 66daaa751e..752cfa60f4 100644
> --- a/sysdeps/mach/hurd/ioctl.c
> +++ b/sysdeps/mach/hurd/ioctl.c
> @@ -28,6 +28,7 @@
>  #include <stdint.h>
>  #include <hurd/ioctl.h>
>  #include <mach/mig_support.h>
> +#include <mach_rpc.h>
>  #include <sysdep-cancel.h>
>  
>  #include <hurd/ioctls.defs>
> @@ -35,11 +36,6 @@
>  #define msg_align(x) ALIGN_UP (x, __alignof__ (uintptr_t))
>  #define typesize(type)	(1 << (type))
>  
> -/* Macro used by MIG to cleanly check the type.  */
> -#define BAD_TYPECHECK(type, check) __glibc_unlikely (({	\
> -  union { mach_msg_type_t t; uint32_t w; } _t, _c;	\
> -  _t.t = *(type); _c.t = *(check);_t.w != _c.w; }))
> -
>  /* Perform the I/O control operation specified by REQUEST on FD.
>     The actual type and use of ARG and the return value depend on REQUEST.  */
>  int
> diff --git a/sysdeps/mach/mach_rpc.h b/sysdeps/mach/mach_rpc.h
> new file mode 100644
> index 0000000000..ff1ee1cae9
> --- /dev/null
> +++ b/sysdeps/mach/mach_rpc.h
> @@ -0,0 +1,31 @@
> +/* Macros for handling Mach RPC messages.
> +   Copyright (C) 2023 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   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 <mach/message.h>
> +
> +/* Macro used by MIG to cleanly check the type.  */
> +#define BAD_TYPECHECK(type, check) __glibc_unlikely (({	\
> +  union { mach_msg_type_t t; uint32_t w; } _t, _c;	\
> +  _t.t = *(type); _c.t = *(check);_t.w != _c.w; }))
> +
> +/* TODO: add this assertion for x86_64.  */
> +#ifndef __x86_64__
> +_Static_assert (sizeof (uint32_t) == sizeof (mach_msg_type_t),
> +                "mach_msg_type_t needs to be the same size as uint32_t");
> +#endif
> +
> -- 
> 2.39.2
>
  

Patch

diff --git a/hurd/hurdselect.c b/hurd/hurdselect.c
index 08fcc92170..9630cae474 100644
--- a/hurd/hurdselect.c
+++ b/hurd/hurdselect.c
@@ -22,6 +22,7 @@ 
 #include <hurd.h>
 #include <hurd/fd.h>
 #include <hurd/io_request.h>
+#include <mach_rpc.h>
 #include <stdlib.h>
 #include <string.h>
 #include <assert.h>
@@ -69,14 +70,6 @@  _hurd_select (int nfds,
   sigset_t oset;
   struct hurd_sigstate *ss = NULL;
 
-  union typeword		/* Use this to avoid unkosher casts.  */
-    {
-      mach_msg_type_t type;
-      uint32_t word;
-    };
-  assert (sizeof (union typeword) == sizeof (mach_msg_type_t));
-  assert (sizeof (uint32_t) == sizeof (mach_msg_type_t));
-
   if (nfds < 0 || (pollfds == NULL && nfds > FD_SETSIZE))
     {
       errno = EINVAL;
@@ -404,15 +397,15 @@  _hurd_select (int nfds,
 	  struct
 	    {
 	      mach_msg_header_t head;
-	      union typeword err_type;
+	      mach_msg_type_t err_type;
 	      error_t err;
 	    } error;
 	  struct
 	    {
 	      mach_msg_header_t head;
-	      union typeword err_type;
+	      mach_msg_type_t err_type;
 	      error_t err;
-	      union typeword result_type;
+	      mach_msg_type_t result_type;
 	      int result;
 	    } success;
 #endif
@@ -443,9 +436,14 @@  _hurd_select (int nfds,
 
 	  /* We got a message.  Decode it.  */
 #ifdef MACH_MSG_TYPE_BIT
-	  const union typeword inttype =
-	  { type:
-	    { MACH_MSG_TYPE_INTEGER_T, sizeof (integer_t) * 8, 1, 1, 0, 0 }
+	  static const mach_msg_type_t inttype = {
+	    .msgt_name = MACH_MSG_TYPE_INTEGER_T,
+	    .msgt_size = sizeof (integer_t) * 8,
+	    .msgt_number = 1,
+	    .msgt_inline = TRUE,
+	    .msgt_longform = FALSE,
+	    .msgt_deallocate = FALSE,
+	    .msgt_unused = 0
 	  };
 #endif
 
@@ -462,7 +460,7 @@  _hurd_select (int nfds,
 	      && msg.head.msgh_size >= sizeof msg.error
 	      && !(msg.head.msgh_bits & MACH_MSGH_BITS_COMPLEX)
 #ifdef MACH_MSG_TYPE_BIT
-	      && msg.error.err_type.word == inttype.word
+	      && !BAD_TYPECHECK (&msg.error.err_type, &inttype)
 #endif
 	      )
 	    {
@@ -480,7 +478,7 @@  _hurd_select (int nfds,
 		 occurred.  */
 	      if (msg.error.err
 #ifdef MACH_MSG_TYPE_BIT
-		  || msg.success.result_type.word != inttype.word
+		  || BAD_TYPECHECK (&msg.success.result_type, &inttype)
 #endif
 		  || msg.head.msgh_size != sizeof msg.success)
 		{
diff --git a/hurd/intr-msg.c b/hurd/intr-msg.c
index b535397bfb..bc1f43d383 100644
--- a/hurd/intr-msg.c
+++ b/hurd/intr-msg.c
@@ -17,6 +17,7 @@ 
    <https://www.gnu.org/licenses/>.  */
 
 #include <mach.h>
+#include <mach_rpc.h>
 #include <mach/mig_errors.h>
 #include <mach/mig_support.h>
 #include <hurd/signal.h>
@@ -28,11 +29,6 @@ 
 # define mig_reply_header_t	mig_reply_error_t
 #endif
 
-/* Macro used by MIG to cleanly check the type.  */
-#define BAD_TYPECHECK(type, check) __glibc_unlikely (({	\
-  union { mach_msg_type_t t; uint32_t w; } _t, _c;	\
-  _t.t = *(type); _c.t = *(check);_t.w != _c.w; }))
-
 error_t
 _hurd_intr_rpc_mach_msg (mach_msg_header_t *msg,
 			 mach_msg_option_t option,
diff --git a/sysdeps/mach/hurd/ioctl.c b/sysdeps/mach/hurd/ioctl.c
index 66daaa751e..752cfa60f4 100644
--- a/sysdeps/mach/hurd/ioctl.c
+++ b/sysdeps/mach/hurd/ioctl.c
@@ -28,6 +28,7 @@ 
 #include <stdint.h>
 #include <hurd/ioctl.h>
 #include <mach/mig_support.h>
+#include <mach_rpc.h>
 #include <sysdep-cancel.h>
 
 #include <hurd/ioctls.defs>
@@ -35,11 +36,6 @@ 
 #define msg_align(x) ALIGN_UP (x, __alignof__ (uintptr_t))
 #define typesize(type)	(1 << (type))
 
-/* Macro used by MIG to cleanly check the type.  */
-#define BAD_TYPECHECK(type, check) __glibc_unlikely (({	\
-  union { mach_msg_type_t t; uint32_t w; } _t, _c;	\
-  _t.t = *(type); _c.t = *(check);_t.w != _c.w; }))
-
 /* Perform the I/O control operation specified by REQUEST on FD.
    The actual type and use of ARG and the return value depend on REQUEST.  */
 int
diff --git a/sysdeps/mach/mach_rpc.h b/sysdeps/mach/mach_rpc.h
new file mode 100644
index 0000000000..ff1ee1cae9
--- /dev/null
+++ b/sysdeps/mach/mach_rpc.h
@@ -0,0 +1,31 @@ 
+/* Macros for handling Mach RPC messages.
+   Copyright (C) 2023 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   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 <mach/message.h>
+
+/* Macro used by MIG to cleanly check the type.  */
+#define BAD_TYPECHECK(type, check) __glibc_unlikely (({	\
+  union { mach_msg_type_t t; uint32_t w; } _t, _c;	\
+  _t.t = *(type); _c.t = *(check);_t.w != _c.w; }))
+
+/* TODO: add this assertion for x86_64.  */
+#ifndef __x86_64__
+_Static_assert (sizeof (uint32_t) == sizeof (mach_msg_type_t),
+                "mach_msg_type_t needs to be the same size as uint32_t");
+#endif
+