[RFC,hurd,6/12] hurd: Fix modes_t and speeds_t types on 64-bit

Message ID 20230212111044.610942-7-bugaevc@gmail.com
State Not applicable, archived
Headers
Series Towards glibc on x86_64-gnu |

Checks

Context Check Description
dj/TryBot-apply_patch fail Patch failed to apply to master at the time it was sent

Commit Message

Sergey Bugaev Feb. 12, 2023, 11:10 a.m. UTC
  ---
 hurd/tioctl.defs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
  

Comments

Samuel Thibault Feb. 12, 2023, 3 p.m. UTC | #1
We don't need these to be 64bits, 32bit will be completely large enough.

Which issue did you actually encounter?

Samuel

Sergey Bugaev, le dim. 12 févr. 2023 14:10:37 +0300, a ecrit:
> ---
>  hurd/tioctl.defs | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hurd/tioctl.defs b/hurd/tioctl.defs
> index 905a4a38..a04f5ae4 100644
> --- a/hurd/tioctl.defs
> +++ b/hurd/tioctl.defs
> @@ -34,9 +34,9 @@ import <hurd/ioctl_types.h>; /* XXX */
>  
>  /* These are the pieces of a struct termios as specified by the
>     definition of _IOT_termios in <termbits.h>. */
> -type modes_t = array[4] of int;
> +type modes_t = array[4] of long;
>  type ccs_t = array[20] of char;
> -type speeds_t = array[2] of int;
> +type speeds_t = array[2] of long;
>  
>  /* This is the arg for a struct winsize as specified by the
>     definition of _IOT_winsize in <sys/ioctl.h>. */
> -- 
> 2.39.1
  
Sergey Bugaev Feb. 12, 2023, 3:15 p.m. UTC | #2
On Sun, Feb 12, 2023 at 6:00 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> We don't need these to be 64bits, 32bit will be completely large enough.
>
> Which issue did you actually encounter?

Well, these MIG definitions need to match the ones in C/glibc. In
hurd/ioctl_types.h (in the Hurd tree) there is

typedef tcflag_t modes_t[4];
typedef speed_t speeds_t[2];

and then in bits/termios.h (in the glibc tree)

/* Type of terminal control flag masks.  */
typedef unsigned long int tcflag_t;

/* Type of control characters.  */
typedef unsigned char cc_t;

/* Type of baud rate specifiers.  */
typedef long int speed_t;

which is why I changed the MIG ones to long. We could do it the other
way around, make our own bits/termios.h and define them as fixed-size
32 bit ints.

Sergey
  
Samuel Thibault Feb. 12, 2023, 3:22 p.m. UTC | #3
Sergey Bugaev, le dim. 12 févr. 2023 18:15:57 +0300, a ecrit:
> On Sun, Feb 12, 2023 at 6:00 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> > We don't need these to be 64bits, 32bit will be completely large enough.
> >
> > Which issue did you actually encounter?
> 
> Well, these MIG definitions need to match the ones in C/glibc. In
> hurd/ioctl_types.h (in the Hurd tree) there is
> 
> typedef tcflag_t modes_t[4];
> typedef speed_t speeds_t[2];
> 
> and then in bits/termios.h (in the glibc tree)
> 
> /* Type of terminal control flag masks.  */
> typedef unsigned long int tcflag_t;
> 
> /* Type of control characters.  */
> typedef unsigned char cc_t;
> 
> /* Type of baud rate specifiers.  */
> typedef long int speed_t;
> 
> which is why I changed the MIG ones to long. We could do it the other
> way around, make our own bits/termios.h and define them as fixed-size
> 32 bit ints.

I'd rather say drop the "long" part, to avoid having to pull the
stdint.h header in. Nowadays' BSD headers just use the int type,
notably.

Samuel
  
Sergey Bugaev Feb. 12, 2023, 4:13 p.m. UTC | #4
On Sun, Feb 12, 2023 at 6:22 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> I'd rather say drop the "long" part, to avoid having to pull the
> stdint.h header in.

That's what I meant, yes.

> Nowadays' BSD headers just use the int type,
> notably.

So given that the Linux port has its own bits/termios.h, is it fine to
just modify the generic one inline, rather than creating a
Hurd-specific version? The patch for that follows.

Sergey

-- >8 --

From 625b774141823a8f504cce8a92b9f45f5e6f050f Mon Sep 17 00:00:00 2001
From: Sergey Bugaev <bugaevc@gmail.com>
Date: Sun, 12 Feb 2023 19:08:57 +0300
Subject: [PATCH] hurd: Fix tcflag_t and speed_t types on 64-bit

These are supposed to stay 32-bit even on 64-bit systems. This matches
BSD and Linux, as well as how these types are already defined in
tioctl.defs

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 bits/termios.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/bits/termios.h b/bits/termios.h
index 4439c2f1..6a883ceb 100644
--- a/bits/termios.h
+++ b/bits/termios.h
@@ -99,13 +99,13 @@
    `tcflag_t', `cc_t', `speed_t' and the `TC*' constants appropriately.  */

 /* Type of terminal control flag masks.  */
-typedef unsigned long int tcflag_t;
+typedef unsigned int tcflag_t;

 /* Type of control characters.  */
 typedef unsigned char cc_t;

 /* Type of baud rate specifiers.  */
-typedef long int speed_t;
+typedef int speed_t;

 /* Terminal control structure.  */
 struct termios
  
Samuel Thibault Feb. 12, 2023, 4:30 p.m. UTC | #5
Sergey Bugaev, le dim. 12 févr. 2023 19:13:58 +0300, a ecrit:
> On Sun, Feb 12, 2023 at 6:22 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> > I'd rather say drop the "long" part, to avoid having to pull the
> > stdint.h header in.
> 
> That's what I meant, yes.
> 
> > Nowadays' BSD headers just use the int type,
> > notably.
> 
> So given that the Linux port has its own bits/termios.h, is it fine to
> just modify the generic one inline, rather than creating a
> Hurd-specific version?

Yes.

> The patch for that follows.
> 
> Sergey
> 
> -- >8 --
> 
> From 625b774141823a8f504cce8a92b9f45f5e6f050f Mon Sep 17 00:00:00 2001
> From: Sergey Bugaev <bugaevc@gmail.com>
> Date: Sun, 12 Feb 2023 19:08:57 +0300
> Subject: [PATCH] hurd: Fix tcflag_t and speed_t types on 64-bit
> 
> These are supposed to stay 32-bit even on 64-bit systems. This matches
> BSD and Linux, as well as how these types are already defined in
> tioctl.defs
> 
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
>  bits/termios.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/bits/termios.h b/bits/termios.h
> index 4439c2f1..6a883ceb 100644
> --- a/bits/termios.h
> +++ b/bits/termios.h
> @@ -99,13 +99,13 @@
>     `tcflag_t', `cc_t', `speed_t' and the `TC*' constants appropriately.  */
> 
>  /* Type of terminal control flag masks.  */
> -typedef unsigned long int tcflag_t;
> +typedef unsigned int tcflag_t;
> 
>  /* Type of control characters.  */
>  typedef unsigned char cc_t;
> 
>  /* Type of baud rate specifiers.  */
> -typedef long int speed_t;
> +typedef int speed_t;
> 
>  /* Terminal control structure.  */
>  struct termios
> -- 
> 2.39.1
>
  
Samuel Thibault Feb. 12, 2023, 7:03 p.m. UTC | #6
Applied, thanks!
Sergey Bugaev, le dim. 12 févr. 2023 19:13:58 +0300, a ecrit:
> On Sun, Feb 12, 2023 at 6:22 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> > I'd rather say drop the "long" part, to avoid having to pull the
> > stdint.h header in.
> 
> That's what I meant, yes.
> 
> > Nowadays' BSD headers just use the int type,
> > notably.
> 
> So given that the Linux port has its own bits/termios.h, is it fine to
> just modify the generic one inline, rather than creating a
> Hurd-specific version? The patch for that follows.
> 
> Sergey
> 
> -- >8 --
> 
> From 625b774141823a8f504cce8a92b9f45f5e6f050f Mon Sep 17 00:00:00 2001
> From: Sergey Bugaev <bugaevc@gmail.com>
> Date: Sun, 12 Feb 2023 19:08:57 +0300
> Subject: [PATCH] hurd: Fix tcflag_t and speed_t types on 64-bit
> 
> These are supposed to stay 32-bit even on 64-bit systems. This matches
> BSD and Linux, as well as how these types are already defined in
> tioctl.defs
> 
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
>  bits/termios.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/bits/termios.h b/bits/termios.h
> index 4439c2f1..6a883ceb 100644
> --- a/bits/termios.h
> +++ b/bits/termios.h
> @@ -99,13 +99,13 @@
>     `tcflag_t', `cc_t', `speed_t' and the `TC*' constants appropriately.  */
> 
>  /* Type of terminal control flag masks.  */
> -typedef unsigned long int tcflag_t;
> +typedef unsigned int tcflag_t;
> 
>  /* Type of control characters.  */
>  typedef unsigned char cc_t;
> 
>  /* Type of baud rate specifiers.  */
> -typedef long int speed_t;
> +typedef int speed_t;
> 
>  /* Terminal control structure.  */
>  struct termios
> -- 
> 2.39.1
>
  

Patch

diff --git a/hurd/tioctl.defs b/hurd/tioctl.defs
index 905a4a38..a04f5ae4 100644
--- a/hurd/tioctl.defs
+++ b/hurd/tioctl.defs
@@ -34,9 +34,9 @@  import <hurd/ioctl_types.h>; /* XXX */
 
 /* These are the pieces of a struct termios as specified by the
    definition of _IOT_termios in <termbits.h>. */
-type modes_t = array[4] of int;
+type modes_t = array[4] of long;
 type ccs_t = array[20] of char;
-type speeds_t = array[2] of int;
+type speeds_t = array[2] of long;
 
 /* This is the arg for a struct winsize as specified by the
    definition of _IOT_winsize in <sys/ioctl.h>. */