PowerPC: Fix termios definitions

Message ID 53DB8C09.3020206@linux.vnet.ibm.com
State Superseded
Headers

Commit Message

Adhemerval Zanella Netto Aug. 1, 2014, 12:46 p.m. UTC
  On 31-07-2014 22:52, Carlos O'Donell wrote:
> On 07/24/2014 09:27 AM, Adhemerval Zanella wrote:
>> This patch fixes the incorrect guard by __USE_MISC of struct winsize and
>> struct termios in powerpc termios header.  Current states leads to build
>> failures if the program defines _XOPEN_SOURCE, but not _DEFAULT_SOURCE
>> or either _BSD_SOURCE or _SVID_SOURCE.  Without any definition,
>> __USE_MISC will not be defined and neither the struct definitions.
> Is struct termios a real problem?
>
> Isn't the minimal fix to just move winsize into the ppc-specific
> ioctl-types.h like it is for all other machines?
>
> I don't care really, moving both makes ioctl-types.h match the
> generic Linux version and that's very good for ppc64le.
>
> OK to checkin if you make it match the generic ioctl-types.h more
> e.g. add the missing comments for winsize.

The issue is in indeed struct winsize being guarded by __USE_MISC in termios.h
header (this needs to be protected by something because it should not
appear in this header) and I moved both to match generic ioctl-types.h.
I have changed it to match the generic one.

>
>> This patches moves powerpc termios.h definitions to powerpc specific
>> ioctl-types.h.  It similar to linux default one, however powerpc
>> struct termio defines ten control characters fields (c_cc), instead of
>> linux default one of eight.  I see a cleanup is possible on this,
>> however due 2.20 release, I took the more conservative approach.
>>
>> It has been reported by Fedora 21 build system [1] and I want to push
>> it to 2.20.  Tested on powerpc64 and powerpc64le.
>>
>> [1] https://bugzilla.redhat.com/show_bug.cgi?id=1122714
>>
>> --
>>
>> 	* sysdeps/unix/sysv/linux/powerpc/bits/ioctl-types.h (struct winsize):
>> 	Moved definition from termios.h.
>> 	(struct termio): Likewise.
>> 	(NCC, TIOCM_LE, TIOCM_DTR, TIOCM_RTS, TIOCM_ST, TIOCM_SR, TIOCM_CTS,
>> 	TIOCM_CAR, TIOCM_RNG, TIOCM_DSR, TIOCM_CD, TIOCM_RI, N_TTY, N_SLIP,
>> 	N_MOUSE, N_PPP, N_STRIP, N_AX25, N_X25, N_6PACK, N_MASC, N_R3964,
>> 	N_PROFIBUS_FDL, N_IRDA, N_SMSBLOCK, N_HDLC, N_SYNC_PPP, N_HCI):
>> 	Likewise.
>> 	* sysdeps/unix/sysv/linux/powerpc/bits/termios.h (struct winsize):
>> 	Moved definition to termios.h.
>> 	(struct termio): Likewise.
>> 	(NCC, TIOCM_LE, TIOCM_DTR, TIOCM_RTS, TIOCM_ST, TIOCM_SR, TIOCM_CTS,
>> 	TIOCM_CAR, TIOCM_RNG, TIOCM_DSR, TIOCM_CD, TIOCM_RI, N_TTY, N_SLIP,
>> 	N_MOUSE, N_PPP, N_STRIP, N_AX25, N_X25, N_6PACK, N_MASC, N_R3964,
>> 	N_PROFIBUS_FDL, N_IRDA, N_SMSBLOCK, N_HDLC, N_SYNC_PPP, N_HCI):
>> 	Likewise.
>>
>> ---
>>
>> diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/ioctl-types.h b/sysdeps/unix/sysv/linux/powerpc/bits/ioctl-types.h
>> index 87b8265..058d4a2 100644
>> --- a/sysdeps/unix/sysv/linux/powerpc/bits/ioctl-types.h
>> +++ b/sysdeps/unix/sysv/linux/powerpc/bits/ioctl-types.h
>> @@ -2,4 +2,57 @@
>>  # error "Never use <bits/ioctl-types.h> directly; include <sys/ioctl.h> instead."
>>  #endif
>>  
>> -#include <termios.h>
>> +/* Get definition of constants for use with `ioctl'.  */
>> +#include <asm/ioctls.h>
>> +
>> +struct winsize
>> +  {
>> +    unsigned short int ws_row;
>> +    unsigned short int ws_col;
>> +    unsigned short int ws_xpixel;
>> +    unsigned short int ws_ypixel;
>> +  };
>> +
> Could you make this match bits/ioctl-types.h as much as
> possible?
>
> That way when I do a diff against bits/ioctl-types.h it 
> shows what's different and what's not.

Certainly.  In my initial patch I tried to change linux default header, but looks
like it would better if powerpc could just use it with some adjust.  This patch
makes powerpc ioctl-types.h use Linux default one with just one adjustment by
changing the n_cc size.  I have checked powerpc64be.

Is that better?

--

	* sysdeps/unix/sysv/linux/bits/ioctl-types.h [NCC]: Set define
	conditionally.
	* sysdeps/unix/sysv/linux/powerpc/bits/ioctl-types.h: Use linux
	default ioctl-types with adjusted NCC value.
	* sysdeps/unix/sysv/linux/powerpc/bits/termios.h (struct winsize):
	Remove definition.
	(struct termio): Likewise.
	(NCC, TIOCM_LE, TIOCM_DTR, TIOCM_RTS, TIOCM_ST, TIOCM_SR, TIOCM_CTS,
	TIOCM_CAR, TIOCM_RNG, TIOCM_DSR, TIOCM_CD, TIOCM_RI, N_TTY, N_SLIP,
	N_MOUSE, N_PPP, N_STRIP, N_AX25, N_X25, N_6PACK, N_MASC, N_R3964,
	N_PROFIBUS_FDL, N_IRDA, N_SMSBLOCK, N_HDLC, N_SYNC_PPP, N_HCI):
	Likewise.

---
  

Comments

Adhemerval Zanella Netto Aug. 1, 2014, 3:09 p.m. UTC | #1
On 01-08-2014 09:46, Adhemerval Zanella wrote:
> On 31-07-2014 22:52, Carlos O'Donell wrote:
>> On 07/24/2014 09:27 AM, Adhemerval Zanella wrote:
>>> This patch fixes the incorrect guard by __USE_MISC of struct winsize and
>>> struct termios in powerpc termios header.  Current states leads to build
>>> failures if the program defines _XOPEN_SOURCE, but not _DEFAULT_SOURCE
>>> or either _BSD_SOURCE or _SVID_SOURCE.  Without any definition,
>>> __USE_MISC will not be defined and neither the struct definitions.
>> Is struct termios a real problem?
>>
>> Isn't the minimal fix to just move winsize into the ppc-specific
>> ioctl-types.h like it is for all other machines?
>>
>> I don't care really, moving both makes ioctl-types.h match the
>> generic Linux version and that's very good for ppc64le.
>>
>> OK to checkin if you make it match the generic ioctl-types.h more
>> e.g. add the missing comments for winsize.
> The issue is in indeed struct winsize being guarded by __USE_MISC in termios.h
> header (this needs to be protected by something because it should not
> appear in this header) and I moved both to match generic ioctl-types.h.
> I have changed it to match the generic one.
>
>>> This patches moves powerpc termios.h definitions to powerpc specific
>>> ioctl-types.h.  It similar to linux default one, however powerpc
>>> struct termio defines ten control characters fields (c_cc), instead of
>>> linux default one of eight.  I see a cleanup is possible on this,
>>> however due 2.20 release, I took the more conservative approach.
>>>
>>> It has been reported by Fedora 21 build system [1] and I want to push
>>> it to 2.20.  Tested on powerpc64 and powerpc64le.
>>>
>>> [1] https://bugzilla.redhat.com/show_bug.cgi?id=1122714
>>>
>>> --
>>>
>>> 	* sysdeps/unix/sysv/linux/powerpc/bits/ioctl-types.h (struct winsize):
>>> 	Moved definition from termios.h.
>>> 	(struct termio): Likewise.
>>> 	(NCC, TIOCM_LE, TIOCM_DTR, TIOCM_RTS, TIOCM_ST, TIOCM_SR, TIOCM_CTS,
>>> 	TIOCM_CAR, TIOCM_RNG, TIOCM_DSR, TIOCM_CD, TIOCM_RI, N_TTY, N_SLIP,
>>> 	N_MOUSE, N_PPP, N_STRIP, N_AX25, N_X25, N_6PACK, N_MASC, N_R3964,
>>> 	N_PROFIBUS_FDL, N_IRDA, N_SMSBLOCK, N_HDLC, N_SYNC_PPP, N_HCI):
>>> 	Likewise.
>>> 	* sysdeps/unix/sysv/linux/powerpc/bits/termios.h (struct winsize):
>>> 	Moved definition to termios.h.
>>> 	(struct termio): Likewise.
>>> 	(NCC, TIOCM_LE, TIOCM_DTR, TIOCM_RTS, TIOCM_ST, TIOCM_SR, TIOCM_CTS,
>>> 	TIOCM_CAR, TIOCM_RNG, TIOCM_DSR, TIOCM_CD, TIOCM_RI, N_TTY, N_SLIP,
>>> 	N_MOUSE, N_PPP, N_STRIP, N_AX25, N_X25, N_6PACK, N_MASC, N_R3964,
>>> 	N_PROFIBUS_FDL, N_IRDA, N_SMSBLOCK, N_HDLC, N_SYNC_PPP, N_HCI):
>>> 	Likewise.
>>>
>>> ---
>>>
>>> diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/ioctl-types.h b/sysdeps/unix/sysv/linux/powerpc/bits/ioctl-types.h
>>> index 87b8265..058d4a2 100644
>>> --- a/sysdeps/unix/sysv/linux/powerpc/bits/ioctl-types.h
>>> +++ b/sysdeps/unix/sysv/linux/powerpc/bits/ioctl-types.h
>>> @@ -2,4 +2,57 @@
>>>  # error "Never use <bits/ioctl-types.h> directly; include <sys/ioctl.h> instead."
>>>  #endif
>>>  
>>> -#include <termios.h>
>>> +/* Get definition of constants for use with `ioctl'.  */
>>> +#include <asm/ioctls.h>
>>> +
>>> +struct winsize
>>> +  {
>>> +    unsigned short int ws_row;
>>> +    unsigned short int ws_col;
>>> +    unsigned short int ws_xpixel;
>>> +    unsigned short int ws_ypixel;
>>> +  };
>>> +
>> Could you make this match bits/ioctl-types.h as much as
>> possible?
>>
>> That way when I do a diff against bits/ioctl-types.h it 
>> shows what's different and what's not.
> Certainly.  In my initial patch I tried to change linux default header, but looks
> like it would better if powerpc could just use it with some adjust.  This patch
> makes powerpc ioctl-types.h use Linux default one with just one adjustment by
> changing the n_cc size.  I have checked powerpc64be.
>
> Is that better?

And I just realized it won't work because the header will be installed and linux default
won't.  I'll will need to copy header's contents and update the n_cc size on powerpc's
one...

>
> --
>
> 	* sysdeps/unix/sysv/linux/bits/ioctl-types.h [NCC]: Set define
> 	conditionally.
> 	* sysdeps/unix/sysv/linux/powerpc/bits/ioctl-types.h: Use linux
> 	default ioctl-types with adjusted NCC value.
> 	* sysdeps/unix/sysv/linux/powerpc/bits/termios.h (struct winsize):
> 	Remove definition.
> 	(struct termio): Likewise.
> 	(NCC, TIOCM_LE, TIOCM_DTR, TIOCM_RTS, TIOCM_ST, TIOCM_SR, TIOCM_CTS,
> 	TIOCM_CAR, TIOCM_RNG, TIOCM_DSR, TIOCM_CD, TIOCM_RI, N_TTY, N_SLIP,
> 	N_MOUSE, N_PPP, N_STRIP, N_AX25, N_X25, N_6PACK, N_MASC, N_R3964,
> 	N_PROFIBUS_FDL, N_IRDA, N_SMSBLOCK, N_HDLC, N_SYNC_PPP, N_HCI):
> 	Likewise.
>
> ---
>
> diff --git a/sysdeps/unix/sysv/linux/bits/ioctl-types.h b/sysdeps/unix/sysv/linux/bits/ioctl-types.h
> index 5b520c5..fb5755a 100644
> --- a/sysdeps/unix/sysv/linux/bits/ioctl-types.h
> +++ b/sysdeps/unix/sysv/linux/bits/ioctl-types.h
> @@ -32,7 +32,9 @@ struct winsize
>      unsigned short int ws_ypixel;
>    };
>
> -#define NCC 8
> +#ifndef NCC
> +# define NCC 8
> +#endif
>  struct termio
>    {
>      unsigned short int c_iflag;		/* input mode flags */
> diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/ioctl-types.h b/sysdeps/unix/sysv/linux/powerpc/bits/ioctl-types.h
> index 87b8265..c7a3341 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/bits/ioctl-types.h
> +++ b/sysdeps/unix/sysv/linux/powerpc/bits/ioctl-types.h
> @@ -1,5 +1,24 @@
> +/* Structure types for pre-termios terminal ioctls.  Linux/powerpc version.
> +   Copyright (C) 2014 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
> +   <http://www.gnu.org/licenses/>.  */
> +
>  #ifndef _SYS_IOCTL_H
>  # error "Never use <bits/ioctl-types.h> directly; include <sys/ioctl.h> instead."
>  #endif
>
> -#include <termios.h>
> +#define NCC 10
> +#include <sysdeps/unix/sysv/linux/bits/ioctl-types.h>
> diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/termios.h b/sysdeps/unix/sysv/linux/powerpc/bits/termios.h
> index b971d3c..bd29608 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/bits/termios.h
> +++ b/sysdeps/unix/sysv/linux/powerpc/bits/termios.h
> @@ -226,32 +226,6 @@ struct termios {
>
>  #ifdef __USE_MISC
>
> -struct sgttyb {
> -	char	sg_ispeed;
> -	char	sg_ospeed;
> -	char	sg_erase;
> -	char	sg_kill;
> -	short	sg_flags;
> -};
> -
> -struct tchars {
> -	char	t_intrc;
> -	char	t_quitc;
> -	char	t_startc;
> -	char	t_stopc;
> -	char	t_eofc;
> -	char	t_brkc;
> -};
> -
> -struct ltchars {
> -	char	t_suspc;
> -	char	t_dsuspc;
> -	char	t_rprntc;
> -	char	t_flushc;
> -	char	t_werasc;
> -	char	t_lnextc;
> -};
> -
>  /* Used for packet mode */
>  #define TIOCPKT_DATA		 0
>  #define TIOCPKT_FLUSHREAD	 1
> @@ -261,24 +235,6 @@ struct ltchars {
>  #define TIOCPKT_NOSTOP		16
>  #define TIOCPKT_DOSTOP		32
>
> -
> -struct winsize {
> -	unsigned short ws_row;
> -	unsigned short ws_col;
> -	unsigned short ws_xpixel;
> -	unsigned short ws_ypixel;
> -};
> -
> -#define NCC 10
> -struct termio {
> -	unsigned short c_iflag;		/* input mode flags */
> -	unsigned short c_oflag;		/* output mode flags */
> -	unsigned short c_cflag;		/* control mode flags */
> -	unsigned short c_lflag;		/* local mode flags */
> -	unsigned char c_line;		/* line discipline */
> -	unsigned char c_cc[NCC];	/* control characters */
> -};
> -
>  /* c_cc characters */
>  #define _VINTR	0
>  #define _VQUIT	1
> @@ -291,38 +247,7 @@ struct termio {
>  #define _VEOL2	8
>  #define _VSWTC	9
>
> -/* modem lines */
> -#define TIOCM_LE	0x001
> -#define TIOCM_DTR	0x002
> -#define TIOCM_RTS	0x004
> -#define TIOCM_ST	0x008
> -#define TIOCM_SR	0x010
> -#define TIOCM_CTS	0x020
> -#define TIOCM_CAR	0x040
> -#define TIOCM_RNG	0x080
> -#define TIOCM_DSR	0x100
> -#define TIOCM_CD	TIOCM_CAR
> -#define TIOCM_RI	TIOCM_RNG
> -
>  /* ioctl (fd, TIOCSERGETLSR, &result) where result may be as below */
>  #define TIOCSER_TEMT    0x01	/* Transmitter physically empty */
>
> -/* line disciplines */
> -#define N_TTY		0
> -#define N_SLIP		1
> -#define N_MOUSE		2
> -#define N_PPP		3
> -#define N_STRIP		4
> -#define N_AX25		5
> -#define N_X25		6	/* X.25 async  */
> -#define N_6PACK		7
> -#define N_MASC		8	/* Mobitex module  */
> -#define N_R3964		9	/* Simatic R3964 module  */
> -#define N_PROFIBUS_FDL	10	/* Profibus  */
> -#define N_IRDA		11	/* Linux IR  */
> -#define N_SMSBLOCK	12	/* SMS block mode  */
> -#define N_HDLC		13	/* synchronous HDLC  */
> -#define N_SYNC_PPP	14	/* synchronous PPP  */
> -#define	N_HCI		15	/* Bluetooth HCI UART  */
> -
>  #endif /* __USE_MISC  */
>
  

Patch

diff --git a/sysdeps/unix/sysv/linux/bits/ioctl-types.h b/sysdeps/unix/sysv/linux/bits/ioctl-types.h
index 5b520c5..fb5755a 100644
--- a/sysdeps/unix/sysv/linux/bits/ioctl-types.h
+++ b/sysdeps/unix/sysv/linux/bits/ioctl-types.h
@@ -32,7 +32,9 @@  struct winsize
     unsigned short int ws_ypixel;
   };
 
-#define NCC 8
+#ifndef NCC
+# define NCC 8
+#endif
 struct termio
   {
     unsigned short int c_iflag;		/* input mode flags */
diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/ioctl-types.h b/sysdeps/unix/sysv/linux/powerpc/bits/ioctl-types.h
index 87b8265..c7a3341 100644
--- a/sysdeps/unix/sysv/linux/powerpc/bits/ioctl-types.h
+++ b/sysdeps/unix/sysv/linux/powerpc/bits/ioctl-types.h
@@ -1,5 +1,24 @@ 
+/* Structure types for pre-termios terminal ioctls.  Linux/powerpc version.
+   Copyright (C) 2014 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
+   <http://www.gnu.org/licenses/>.  */
+
 #ifndef _SYS_IOCTL_H
 # error "Never use <bits/ioctl-types.h> directly; include <sys/ioctl.h> instead."
 #endif
 
-#include <termios.h>
+#define NCC 10
+#include <sysdeps/unix/sysv/linux/bits/ioctl-types.h>
diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/termios.h b/sysdeps/unix/sysv/linux/powerpc/bits/termios.h
index b971d3c..bd29608 100644
--- a/sysdeps/unix/sysv/linux/powerpc/bits/termios.h
+++ b/sysdeps/unix/sysv/linux/powerpc/bits/termios.h
@@ -226,32 +226,6 @@  struct termios {
 
 #ifdef __USE_MISC
 
-struct sgttyb {
-	char	sg_ispeed;
-	char	sg_ospeed;
-	char	sg_erase;
-	char	sg_kill;
-	short	sg_flags;
-};
-
-struct tchars {
-	char	t_intrc;
-	char	t_quitc;
-	char	t_startc;
-	char	t_stopc;
-	char	t_eofc;
-	char	t_brkc;
-};
-
-struct ltchars {
-	char	t_suspc;
-	char	t_dsuspc;
-	char	t_rprntc;
-	char	t_flushc;
-	char	t_werasc;
-	char	t_lnextc;
-};
-
 /* Used for packet mode */
 #define TIOCPKT_DATA		 0
 #define TIOCPKT_FLUSHREAD	 1
@@ -261,24 +235,6 @@  struct ltchars {
 #define TIOCPKT_NOSTOP		16
 #define TIOCPKT_DOSTOP		32
 
-
-struct winsize {
-	unsigned short ws_row;
-	unsigned short ws_col;
-	unsigned short ws_xpixel;
-	unsigned short ws_ypixel;
-};
-
-#define NCC 10
-struct termio {
-	unsigned short c_iflag;		/* input mode flags */
-	unsigned short c_oflag;		/* output mode flags */
-	unsigned short c_cflag;		/* control mode flags */
-	unsigned short c_lflag;		/* local mode flags */
-	unsigned char c_line;		/* line discipline */
-	unsigned char c_cc[NCC];	/* control characters */
-};
-
 /* c_cc characters */
 #define _VINTR	0
 #define _VQUIT	1
@@ -291,38 +247,7 @@  struct termio {
 #define _VEOL2	8
 #define _VSWTC	9
 
-/* modem lines */
-#define TIOCM_LE	0x001
-#define TIOCM_DTR	0x002
-#define TIOCM_RTS	0x004
-#define TIOCM_ST	0x008
-#define TIOCM_SR	0x010
-#define TIOCM_CTS	0x020
-#define TIOCM_CAR	0x040
-#define TIOCM_RNG	0x080
-#define TIOCM_DSR	0x100
-#define TIOCM_CD	TIOCM_CAR
-#define TIOCM_RI	TIOCM_RNG
-
 /* ioctl (fd, TIOCSERGETLSR, &result) where result may be as below */
 #define TIOCSER_TEMT    0x01	/* Transmitter physically empty */
 
-/* line disciplines */
-#define N_TTY		0
-#define N_SLIP		1
-#define N_MOUSE		2
-#define N_PPP		3
-#define N_STRIP		4
-#define N_AX25		5
-#define N_X25		6	/* X.25 async  */
-#define N_6PACK		7
-#define N_MASC		8	/* Mobitex module  */
-#define N_R3964		9	/* Simatic R3964 module  */
-#define N_PROFIBUS_FDL	10	/* Profibus  */
-#define N_IRDA		11	/* Linux IR  */
-#define N_SMSBLOCK	12	/* SMS block mode  */
-#define N_HDLC		13	/* synchronous HDLC  */
-#define N_SYNC_PPP	14	/* synchronous PPP  */
-#define	N_HCI		15	/* Bluetooth HCI UART  */
-
 #endif /* __USE_MISC  */