[2/3] Move register_t to system-specific header

Message ID 20190213131912.20445-2-sebastian.huber@embedded-brains.de
State New
Headers show
Series
  • [1/3] Move RTEMS and XMK specific type definitions
Related show

Commit Message

Sebastian Huber Feb. 13, 2019, 1:19 p.m.
Signed-off-by: Sebastian Huber <sebastian.huber@embedded-brains.de>

---
 newlib/libc/include/machine/types.h           | 2 ++
 newlib/libc/include/sys/types.h               | 2 +-
 newlib/libc/sys/rtems/include/machine/types.h | 2 ++
 winsup/cygwin/include/machine/types.h         | 2 ++
 4 files changed, 7 insertions(+), 1 deletion(-)

-- 
2.16.4

Comments

Corinna Vinschen Feb. 13, 2019, 5:06 p.m. | #1
On Feb 13 14:19, Sebastian Huber wrote:
> Signed-off-by: Sebastian Huber <sebastian.huber@embedded-brains.de>

> ---

>  newlib/libc/include/machine/types.h           | 2 ++

>  newlib/libc/include/sys/types.h               | 2 +-

>  newlib/libc/sys/rtems/include/machine/types.h | 2 ++

>  winsup/cygwin/include/machine/types.h         | 2 ++

>  4 files changed, 7 insertions(+), 1 deletion(-)

> 

> diff --git a/newlib/libc/include/machine/types.h b/newlib/libc/include/machine/types.h

> index 19d0e8560..fab9cf72c 100644

> --- a/newlib/libc/include/machine/types.h

> +++ b/newlib/libc/include/machine/types.h

> @@ -11,3 +11,5 @@ typedef	__uint64_t	u_quad_t;

>  typedef	__int64_t	quad_t;

>  typedef	quad_t *	qaddr_t;

>  #endif

> +

> +typedef int register_t;

> diff --git a/newlib/libc/include/sys/types.h b/newlib/libc/include/sys/types.h

> index 2685df654..e05263d4e 100644

> --- a/newlib/libc/include/sys/types.h

> +++ b/newlib/libc/include/sys/types.h

> @@ -36,7 +36,7 @@ typedef __uint32_t	u_int32_t;

>  #if ___int64_t_defined

>  typedef __uint64_t	u_int64_t;

>  #endif

> -typedef int register_t;

> +

>  #define __BIT_TYPES_DEFINED__ 1


Why move this out here?  Sure, it's wrong for 64 bit targets ATM,
but moving it to rtems and Cygwin only means that the type suddenly
disappears for other targets.  Better just fix it here for all targets,
including defining uregister_t if __BSD_VISIBLE.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat
Sebastian Huber Feb. 13, 2019, 7:01 p.m. | #2
----- Am 13. Feb 2019 um 18:06 schrieb Corinna Vinschen vinschen@redhat.com:

> On Feb 13 14:19, Sebastian Huber wrote:

>> Signed-off-by: Sebastian Huber <sebastian.huber@embedded-brains.de>

>> ---

>>  newlib/libc/include/machine/types.h           | 2 ++

>>  newlib/libc/include/sys/types.h               | 2 +-

>>  newlib/libc/sys/rtems/include/machine/types.h | 2 ++

>>  winsup/cygwin/include/machine/types.h         | 2 ++

>>  4 files changed, 7 insertions(+), 1 deletion(-)

>> 

>> diff --git a/newlib/libc/include/machine/types.h

>> b/newlib/libc/include/machine/types.h

>> index 19d0e8560..fab9cf72c 100644

>> --- a/newlib/libc/include/machine/types.h

>> +++ b/newlib/libc/include/machine/types.h

>> @@ -11,3 +11,5 @@ typedef	__uint64_t	u_quad_t;

>>  typedef	__int64_t	quad_t;

>>  typedef	quad_t *	qaddr_t;

>>  #endif

>> +

>> +typedef int register_t;

>> diff --git a/newlib/libc/include/sys/types.h b/newlib/libc/include/sys/types.h

>> index 2685df654..e05263d4e 100644

>> --- a/newlib/libc/include/sys/types.h

>> +++ b/newlib/libc/include/sys/types.h

>> @@ -36,7 +36,7 @@ typedef __uint32_t	u_int32_t;

>>  #if ___int64_t_defined

>>  typedef __uint64_t	u_int64_t;

>>  #endif

>> -typedef int register_t;

>> +

>>  #define __BIT_TYPES_DEFINED__ 1

> 

> Why move this out here?  Sure, it's wrong for 64 bit targets ATM,

> but moving it to rtems and Cygwin only means that the type suddenly

> disappears for other targets.  Better just fix it here for all targets,

> including defining uregister_t if __BSD_VISIBLE.


I found three <machine/types.h> in Newlib, one for Cygwin, one for RTEMS and one for the rest. I moved the definition of register_t to the <machine/types.h> which is included by <sys/types.h> at the end.

If I change the type to __intptr_t wounldn't this break ABI compatibility on Cygwin?
Corinna Vinschen Feb. 13, 2019, 8:28 p.m. | #3
On Feb 13 20:01, Sebastian Huber wrote:
> ----- Am 13. Feb 2019 um 18:06 schrieb Corinna Vinschen vinschen@redhat.com:

> 

> > On Feb 13 14:19, Sebastian Huber wrote:

> >> Signed-off-by: Sebastian Huber <sebastian.huber@embedded-brains.de>

> >> ---

> >>  newlib/libc/include/machine/types.h           | 2 ++

> >>  newlib/libc/include/sys/types.h               | 2 +-

> >>  newlib/libc/sys/rtems/include/machine/types.h | 2 ++

> >>  winsup/cygwin/include/machine/types.h         | 2 ++

> >>  4 files changed, 7 insertions(+), 1 deletion(-)

> >> 

> >> diff --git a/newlib/libc/include/machine/types.h

> >> b/newlib/libc/include/machine/types.h

> >> index 19d0e8560..fab9cf72c 100644

> >> --- a/newlib/libc/include/machine/types.h

> >> +++ b/newlib/libc/include/machine/types.h

> >> @@ -11,3 +11,5 @@ typedef	__uint64_t	u_quad_t;

> >>  typedef	__int64_t	quad_t;

> >>  typedef	quad_t *	qaddr_t;

> >>  #endif

> >> +

> >> +typedef int register_t;

> >> diff --git a/newlib/libc/include/sys/types.h b/newlib/libc/include/sys/types.h

> >> index 2685df654..e05263d4e 100644

> >> --- a/newlib/libc/include/sys/types.h

> >> +++ b/newlib/libc/include/sys/types.h

> >> @@ -36,7 +36,7 @@ typedef __uint32_t	u_int32_t;

> >>  #if ___int64_t_defined

> >>  typedef __uint64_t	u_int64_t;

> >>  #endif

> >> -typedef int register_t;

> >> +

> >>  #define __BIT_TYPES_DEFINED__ 1

> > 

> > Why move this out here?  Sure, it's wrong for 64 bit targets ATM,

> > but moving it to rtems and Cygwin only means that the type suddenly

> > disappears for other targets.  Better just fix it here for all targets,

> > including defining uregister_t if __BSD_VISIBLE.

> 

> I found three <machine/types.h> in Newlib, one for Cygwin, one for

> RTEMS and one for the rest. I moved the definition of register_t to

> the <machine/types.h> which is included by <sys/types.h> at the end.


Oh, right, I missed the non-rtems, non-Cygwin case.

> If I change the type to __intptr_t wounldn't this break ABI

> compatibility on Cygwin?


In how far?  Cygwin's 64 bit ABI is LP64.


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat
Sebastian Huber Feb. 14, 2019, 10:15 a.m. | #4
----- Am 13. Feb 2019 um 21:28 schrieb Corinna Vinschen vinschen@redhat.com:

> On Feb 13 20:01, Sebastian Huber wrote:

>> ----- Am 13. Feb 2019 um 18:06 schrieb Corinna Vinschen vinschen@redhat.com:

>> 

>> > On Feb 13 14:19, Sebastian Huber wrote:

[...]
>> If I change the type to __intptr_t wounldn't this break ABI

>> compatibility on Cygwin?

> 

> In how far?  Cygwin's 64 bit ABI is LP64.


The type for register_t changes from int to __intptr_to, so 32-bit to 64-bit on LP64.
Corinna Vinschen Feb. 14, 2019, 2:02 p.m. | #5
On Feb 14 11:15, Sebastian Huber wrote:
> 

> 

> ----- Am 13. Feb 2019 um 21:28 schrieb Corinna Vinschen vinschen@redhat.com:

> 

> > On Feb 13 20:01, Sebastian Huber wrote:

> >> ----- Am 13. Feb 2019 um 18:06 schrieb Corinna Vinschen vinschen@redhat.com:

> >> 

> >> > On Feb 13 14:19, Sebastian Huber wrote:

> [...]

> >> If I change the type to __intptr_t wounldn't this break ABI

> >> compatibility on Cygwin?

> > 

> > In how far?  Cygwin's 64 bit ABI is LP64.

> 

> The type for register_t changes from int to __intptr_to, so 32-bit to 64-bit on LP64.


register_t is not used in Cygwin itself.  I don't know its purpose,
actually.  If it has been defined as 32 bit type on 64 bit, wasn't that
a bug and thus unusable before?


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat
Sebastian Huber Feb. 14, 2019, 6:50 p.m. | #6
----- Am 14. Feb 2019 um 15:02 schrieb Corinna Vinschen vinschen@redhat.com:

> On Feb 14 11:15, Sebastian Huber wrote:

>> 

>> 

>> ----- Am 13. Feb 2019 um 21:28 schrieb Corinna Vinschen vinschen@redhat.com:

>> 

>> > On Feb 13 20:01, Sebastian Huber wrote:

>> >> ----- Am 13. Feb 2019 um 18:06 schrieb Corinna Vinschen vinschen@redhat.com:

>> >> 

>> >> > On Feb 13 14:19, Sebastian Huber wrote:

>> [...]

>> >> If I change the type to __intptr_t wounldn't this break ABI

>> >> compatibility on Cygwin?

>> > 

>> > In how far?  Cygwin's 64 bit ABI is LP64.

>> 

>> The type for register_t changes from int to __intptr_to, so 32-bit to 64-bit on

>> LP64.

> 

> register_t is not used in Cygwin itself.  I don't know its purpose,

> actually.  If it has been defined as 32 bit type on 64 bit, wasn't that

> a bug and thus unusable before?


Yes, a 32-bit register_t type on an LP64 system is a bug. It is used in some situations like intptr_t in FreeBSD.
Corinna Vinschen Feb. 14, 2019, 7:19 p.m. | #7
On Feb 14 19:50, Sebastian Huber wrote:
> ----- Am 14. Feb 2019 um 15:02 schrieb Corinna Vinschen vinschen@redhat.com:

> 

> > On Feb 14 11:15, Sebastian Huber wrote:

> >> 

> >> 

> >> ----- Am 13. Feb 2019 um 21:28 schrieb Corinna Vinschen vinschen@redhat.com:

> >> 

> >> > On Feb 13 20:01, Sebastian Huber wrote:

> >> >> ----- Am 13. Feb 2019 um 18:06 schrieb Corinna Vinschen vinschen@redhat.com:

> >> >> 

> >> >> > On Feb 13 14:19, Sebastian Huber wrote:

> >> [...]

> >> >> If I change the type to __intptr_t wounldn't this break ABI

> >> >> compatibility on Cygwin?

> >> > 

> >> > In how far?  Cygwin's 64 bit ABI is LP64.

> >> 

> >> The type for register_t changes from int to __intptr_to, so 32-bit to 64-bit on

> >> LP64.

> > 

> > register_t is not used in Cygwin itself.  I don't know its purpose,

> > actually.  If it has been defined as 32 bit type on 64 bit, wasn't that

> > a bug and thus unusable before?

> 

> Yes, a 32-bit register_t type on an LP64 system is a bug. It is used

> in some situations like intptr_t in FreeBSD.


Then it's ok to change it generically in just one spot.  Adding
uregister_t under BSD_VISIBLE as well.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

Patch

diff --git a/newlib/libc/include/machine/types.h b/newlib/libc/include/machine/types.h
index 19d0e8560..fab9cf72c 100644
--- a/newlib/libc/include/machine/types.h
+++ b/newlib/libc/include/machine/types.h
@@ -11,3 +11,5 @@  typedef	__uint64_t	u_quad_t;
 typedef	__int64_t	quad_t;
 typedef	quad_t *	qaddr_t;
 #endif
+
+typedef int register_t;
diff --git a/newlib/libc/include/sys/types.h b/newlib/libc/include/sys/types.h
index 2685df654..e05263d4e 100644
--- a/newlib/libc/include/sys/types.h
+++ b/newlib/libc/include/sys/types.h
@@ -36,7 +36,7 @@  typedef __uint32_t	u_int32_t;
 #if ___int64_t_defined
 typedef __uint64_t	u_int64_t;
 #endif
-typedef int register_t;
+
 #define __BIT_TYPES_DEFINED__ 1
 
 #ifndef __need_inttypes
diff --git a/newlib/libc/sys/rtems/include/machine/types.h b/newlib/libc/sys/rtems/include/machine/types.h
index c550873d3..54c643016 100644
--- a/newlib/libc/sys/rtems/include/machine/types.h
+++ b/newlib/libc/sys/rtems/include/machine/types.h
@@ -68,6 +68,8 @@  typedef	__int64_t	quad_t;
 typedef	quad_t *	qaddr_t;
 #endif
 
+typedef	int	register_t;
+
 #ifndef _RLIM_T_DECLARED
 typedef	__rlim_t	rlim_t;		/* resource limit */
 #define	_RLIM_T_DECLARED
diff --git a/winsup/cygwin/include/machine/types.h b/winsup/cygwin/include/machine/types.h
index 54b4acf0a..0ef476b17 100644
--- a/winsup/cygwin/include/machine/types.h
+++ b/winsup/cygwin/include/machine/types.h
@@ -51,6 +51,8 @@  struct flock {
 	pid_t	 l_pid;		/* returned with F_GETLK */
 };
 
+typedef int register_t;
+
 #ifndef __BIT_TYPES_DEFINED
 #define __BIT_TYPES_DEFINED__ 1