[PATCH] Fix type inconsistencies in stdint.h

classic Classic list List threaded Threaded
32 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[PATCH] Fix type inconsistencies in stdint.h

Dave Korn-6

    Hi team,

  Upstream GCC just gained the ability to know all about the stdint.h types
and limits internally.  If you're interested in the background, see

    http://gcc.gnu.org/ml/gcc/2009-04/msg00000.html

and thread for further reading.

  I've submitted the necessary info for the cygwin GCC backend, and now the
associated testcases show up a few bugs in our stdint.h declarations.
Basically:-

- uint32_t is "unsigned int", but UINT32_MAX is an unsigned long int constant
(denoted by the 'UL' suffix).
- int_least32_t is "long", where INT_LEAST32_MIN and INT_LEAST32_MAX are plain
(unsuffixed) int constants.
- int_fast16_t and int_fast32_t are both "long", where INT_FAST16/32_MIN/MAX
are all plain (unsuffixed) ints.
- intptr_t is "long" but INTPTR_MIN and INTPTR_MAX lack the "L" suffix and so
are just ints.
- size_t is "unsigned int" but the SIZE_MAX constant is unsigned long.

  This is bad because if the value of one of these MIN or MAX limits is not of
the correct integer type matching the integer type it is used in conjunction
with, there will be an implicit cast operation anytime you assign the
wrongly-typed value to a variable of the type for which it is supposed to be
the limit.

  The attached patch fixes all these by adjusting only the suffix letters.  OK
for head?

winsup/cygwin/ChangeLog

        * include/stdint.h (UINT32_MAX, INT_LEAST32_MIN, INT_LEAST32_MAX,
        INT_FAST16_MIN, INT_FAST32_MIN, INT_FAST16_MAX, INT_FAST32_MAX,
        INTPTR_MIN, INTPTR_MAX, SIZE_MAX):  Fix integer constant suffixes.

    cheers,
      DaveK

Index: winsup/cygwin/include/stdint.h
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/include/stdint.h,v
retrieving revision 1.10
diff -p -u -r1.10 stdint.h
--- winsup/cygwin/include/stdint.h 17 May 2008 21:34:05 -0000 1.10
+++ winsup/cygwin/include/stdint.h 3 Apr 2009 23:07:50 -0000
@@ -80,19 +80,19 @@ typedef unsigned long long uintmax_t;
 
 #define UINT8_MAX (255)
 #define UINT16_MAX (65535)
-#define UINT32_MAX (4294967295UL)
+#define UINT32_MAX (4294967295U)
 #define UINT64_MAX (18446744073709551615ULL)
 
 /* Limits of minimum-width integer types */
 
 #define INT_LEAST8_MIN (-128)
 #define INT_LEAST16_MIN (-32768)
-#define INT_LEAST32_MIN (-2147483647 - 1)
+#define INT_LEAST32_MIN (-2147483647L - 1L)
 #define INT_LEAST64_MIN (-9223372036854775807LL - 1LL)
 
 #define INT_LEAST8_MAX (127)
 #define INT_LEAST16_MAX (32767)
-#define INT_LEAST32_MAX (2147483647)
+#define INT_LEAST32_MAX (2147483647L)
 #define INT_LEAST64_MAX (9223372036854775807LL)
 
 #define UINT_LEAST8_MAX (255)
@@ -103,13 +103,13 @@ typedef unsigned long long uintmax_t;
 /* Limits of fastest minimum-width integer types */
 
 #define INT_FAST8_MIN (-128)
-#define INT_FAST16_MIN (-2147483647 - 1)
-#define INT_FAST32_MIN (-2147483647 - 1)
+#define INT_FAST16_MIN (-2147483647L - 1L)
+#define INT_FAST32_MIN (-2147483647L - 1L)
 #define INT_FAST64_MIN (-9223372036854775807LL - 1LL)
 
 #define INT_FAST8_MAX (127)
-#define INT_FAST16_MAX (2147483647)
-#define INT_FAST32_MAX (2147483647)
+#define INT_FAST16_MAX (2147483647L)
+#define INT_FAST32_MAX (2147483647L)
 #define INT_FAST64_MAX (9223372036854775807LL)
 
 #define UINT_FAST8_MAX (255)
@@ -119,8 +119,8 @@ typedef unsigned long long uintmax_t;
 
 /* Limits of integer types capable of holding object pointers */
 
-#define INTPTR_MIN (-2147483647 - 1)
-#define INTPTR_MAX (2147483647)
+#define INTPTR_MIN (-2147483647L - 1L)
+#define INTPTR_MAX (2147483647L)
 #define UINTPTR_MAX (4294967295UL)
 
 /* Limits of greatest-width integer types */
@@ -144,7 +144,7 @@ typedef unsigned long long uintmax_t;
 #endif
 
 #ifndef SIZE_MAX
-#define SIZE_MAX (4294967295UL)
+#define SIZE_MAX (4294967295U)
 #endif
 
 #ifndef WCHAR_MIN
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix type inconsistencies in stdint.h

Christopher Faylor-8
On Sat, Apr 04, 2009 at 02:33:11AM +0100, Dave Korn wrote:

>
>    Hi team,
>
>  Upstream GCC just gained the ability to know all about the stdint.h types
>and limits internally.  If you're interested in the background, see
>
>    http://gcc.gnu.org/ml/gcc/2009-04/msg00000.html
>
>and thread for further reading.
>
>  I've submitted the necessary info for the cygwin GCC backend, and now the
>associated testcases show up a few bugs in our stdint.h declarations.
>Basically:-
>
>- uint32_t is "unsigned int", but UINT32_MAX is an unsigned long int constant
>(denoted by the 'UL' suffix).
>- int_least32_t is "long", where INT_LEAST32_MIN and INT_LEAST32_MAX are plain
>(unsuffixed) int constants.
>- int_fast16_t and int_fast32_t are both "long", where INT_FAST16/32_MIN/MAX
>are all plain (unsuffixed) ints.
>- intptr_t is "long" but INTPTR_MIN and INTPTR_MAX lack the "L" suffix and so
>are just ints.
>- size_t is "unsigned int" but the SIZE_MAX constant is unsigned long.
>
>  This is bad because if the value of one of these MIN or MAX limits is not of
>the correct integer type matching the integer type it is used in conjunction
>with, there will be an implicit cast operation anytime you assign the
>wrongly-typed value to a variable of the type for which it is supposed to be
>the limit.
>
>  The attached patch fixes all these by adjusting only the suffix letters.  OK
>for head?
>
>winsup/cygwin/ChangeLog
>
> * include/stdint.h (UINT32_MAX, INT_LEAST32_MIN, INT_LEAST32_MAX,
> INT_FAST16_MIN, INT_FAST32_MIN, INT_FAST16_MAX, INT_FAST32_MAX,
> INTPTR_MIN, INTPTR_MAX, SIZE_MAX):  Fix integer constant suffixes.

Many of the changes introduce divergence from Linux.  Why is that?

cgf
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix type inconsistencies in stdint.h

Dave Korn-6
Christopher Faylor wrote:

>>  The attached patch fixes all these by adjusting only the suffix letters.  OK
>> for head?
>>
>> winsup/cygwin/ChangeLog
>>
>> * include/stdint.h (UINT32_MAX, INT_LEAST32_MIN, INT_LEAST32_MAX,
>> INT_FAST16_MIN, INT_FAST32_MIN, INT_FAST16_MAX, INT_FAST32_MAX,
>> INTPTR_MIN, INTPTR_MAX, SIZE_MAX):  Fix integer constant suffixes.
>
> Many of the changes introduce divergence from Linux.  Why is that?

  Because our stdint.h types are divergent from Linux, and changing them
instead could cause yet another ABI break.

    cheers,
      DaveK

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix type inconsistencies in stdint.h

Christopher Faylor-8
On Sat, Apr 04, 2009 at 05:11:09AM +0100, Dave Korn wrote:

>Christopher Faylor wrote:
>
>>>  The attached patch fixes all these by adjusting only the suffix letters.  OK
>>> for head?
>>>
>>> winsup/cygwin/ChangeLog
>>>
>>> * include/stdint.h (UINT32_MAX, INT_LEAST32_MIN, INT_LEAST32_MAX,
>>> INT_FAST16_MIN, INT_FAST32_MIN, INT_FAST16_MAX, INT_FAST32_MAX,
>>> INTPTR_MIN, INTPTR_MAX, SIZE_MAX):  Fix integer constant suffixes.
>>
>>Many of the changes introduce divergence from Linux.  Why is that?
>
>Because our stdint.h types are divergent from Linux, and changing them
>instead could cause yet another ABI break.

Why would changing uint32_t from 'unsigned long' to 'unsigned int' break
anything?  It looks to me like that is a disaster waiting to happen if
we ever provide a 64-bit port.

Isn't a long 32 bits?  What would be the ABI breakage in changing that
one typedef rather than lots of #defines?  It seems like fixing the
typedefs in stdint.h is the right thing to do before Cygwin 1.7 rolls
out.

cgf
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix type inconsistencies in stdint.h

Christopher Faylor-8
On Sat, Apr 04, 2009 at 02:24:59AM -0400, Christopher Faylor wrote:

>On Sat, Apr 04, 2009 at 05:11:09AM +0100, Dave Korn wrote:
>>Christopher Faylor wrote:
>>
>>>>  The attached patch fixes all these by adjusting only the suffix letters.  OK
>>>> for head?
>>>>
>>>> winsup/cygwin/ChangeLog
>>>>
>>>> * include/stdint.h (UINT32_MAX, INT_LEAST32_MIN, INT_LEAST32_MAX,
>>>> INT_FAST16_MIN, INT_FAST32_MIN, INT_FAST16_MAX, INT_FAST32_MAX,
>>>> INTPTR_MIN, INTPTR_MAX, SIZE_MAX):  Fix integer constant suffixes.
>>>
>>>Many of the changes introduce divergence from Linux.  Why is that?
>>
>>Because our stdint.h types are divergent from Linux, and changing them
>>instead could cause yet another ABI break.
>
>Why would changing uint32_t from 'unsigned long' to 'unsigned int' break
>anything?  It looks to me like that is a disaster waiting to happen if
>we ever provide a 64-bit port.

The disaster I'm referring to, in case it isn't clear, is keeping the
current typedef.

cgf
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix type inconsistencies in stdint.h

Dave Korn-6
In reply to this post by Christopher Faylor-8
Christopher Faylor wrote:

> Why would changing uint32_t from 'unsigned long' to 'unsigned int' break
> anything?  

  Well, it mangles differently and will resolve overloads and instantiate
templates differently won't it?

> It looks to me like that is a disaster waiting to happen if
> we ever provide a 64-bit port

  Not necessarily.  Win64 is an LLP64 platform.  Won't that make it OK?  All
the 32-bit integer types remain 32-bits, the 64-bit long long remains 64-bits,
only pointers change in size.  (Along with the related [u]intptr_t, ptrdiff_t
etc.)

> Isn't a long 32 bits?  What would be the ABI breakage in changing that
> one typedef rather than lots of #defines?  It seems like fixing the
> typedefs in stdint.h is the right thing to do before Cygwin 1.7 rolls
> out.

  Agreed, but I'm not sure if they are wrong yet.  Just different, with a
legacy of applications that expect them to be the way they currently are.

    cheers,
      DaveK
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix type inconsistencies in stdint.h

Dave Korn-6
In reply to this post by Christopher Faylor-8
Christopher Faylor wrote:

  Ah, I could address a bit more to these two questions as well:

> Isn't a long 32 bits?  What would be the ABI breakage in changing that
> one typedef rather than lots of #defines?  

  Yes, a long is 32 bits, but while that makes for binary ABI
(calling-convention) compatibility it isn't the same thing in the C and C++
types system.  Therefore the underlying types are an inextricably woven part
of the overall C-language ABI as well as their physical bit sizes.  Changing
them certainly has the potential to change the ABI, particularly in C++, but I
think it also might potentially render some of the compiler's aliasing
assumptions invalid when linking code using the new definitions against
objects or libraries using the old.

  Changing the limits #defines, OTOH, is absolutely guaranteed ABI neutral.
They really are "just constants" at runtime, and constants don't get mangled
or alias anything.  So I reckon it's a safer way to proceed and I don't yet
see any potential 64-bit problems down the line if we leave everything as it
currently stands.

  Can you see anything I've overlooked in this analysis?

    cheers,
      DaveK
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix type inconsistencies in stdint.h

Corinna Vinschen-2
On Apr  4 08:23, Dave Korn wrote:

> Christopher Faylor wrote:
>
>   Ah, I could address a bit more to these two questions as well:
>
> > Isn't a long 32 bits?  What would be the ABI breakage in changing that
> > one typedef rather than lots of #defines?  
>
>   Yes, a long is 32 bits, but while that makes for binary ABI
> (calling-convention) compatibility it isn't the same thing in the C and C++
> types system.  Therefore the underlying types are an inextricably woven part
> of the overall C-language ABI as well as their physical bit sizes.  Changing
> them certainly has the potential to change the ABI, particularly in C++, but I
> think it also might potentially render some of the compiler's aliasing
> assumptions invalid when linking code using the new definitions against
> objects or libraries using the old.
>
>   Changing the limits #defines, OTOH, is absolutely guaranteed ABI neutral.
> They really are "just constants" at runtime, and constants don't get mangled
> or alias anything.  So I reckon it's a safer way to proceed and I don't yet
> see any potential 64-bit problems down the line if we leave everything as it
> currently stands.
>
>   Can you see anything I've overlooked in this analysis?

Sounds right to me.  Given thr LLP64-ness of Win64, it should be no
problem to stick to the types.


Corinna

--
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix type inconsistencies in stdint.h

Corinna Vinschen-2
On Apr  4 11:47, Corinna Vinschen wrote:

> On Apr  4 08:23, Dave Korn wrote:
> > Christopher Faylor wrote:
> >
> >   Ah, I could address a bit more to these two questions as well:
> >
> > > Isn't a long 32 bits?  What would be the ABI breakage in changing that
> > > one typedef rather than lots of #defines?  
> >
> >   Yes, a long is 32 bits, but while that makes for binary ABI
> > (calling-convention) compatibility it isn't the same thing in the C and C++
> > types system.  Therefore the underlying types are an inextricably woven part
> > of the overall C-language ABI as well as their physical bit sizes.  Changing
> > them certainly has the potential to change the ABI, particularly in C++, but I
> > think it also might potentially render some of the compiler's aliasing
> > assumptions invalid when linking code using the new definitions against
> > objects or libraries using the old.
> >
> >   Changing the limits #defines, OTOH, is absolutely guaranteed ABI neutral.
> > They really are "just constants" at runtime, and constants don't get mangled
> > or alias anything.  So I reckon it's a safer way to proceed and I don't yet
> > see any potential 64-bit problems down the line if we leave everything as it
> > currently stands.
> >
> >   Can you see anything I've overlooked in this analysis?
>
> Sounds right to me.  Given thr LLP64-ness of Win64, it should be no
> problem to stick to the types.

OTOH, we already had to change int32_t and uint32_t from long to int to
avoid warnings.  Given that we already changed that anyway, I'm wondering
if it isn't more sane to align the least and fast types as well.


Corinna

--
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix type inconsistencies in stdint.h

Dave Korn-6
Corinna Vinschen wrote:

> OTOH, we already had to change int32_t and uint32_t from long to int to
> avoid warnings.  Given that we already changed that anyway, I'm wondering
> if it isn't more sane to align the least and fast types as well.

  Well, if there was ever a time to do it, now would be that time, and I'll
happily go update GCC to accord with whatever we decide to do.  I can't say
what kind of incompatibilities might arise, as it's not an easy thing to
google uses of these types specifically in exported rather than internal APIs.
 It's possible things like codec libraries and heavy graphics number-crunchers
might specify these types in externally-visible definitions but I haven't done
an audit.

  As long as we don't change the size, binaries will still interoperate fine,
except where changed name-mangling prevents linking, but e.g. structs and wire
or file formats will remain unchanged.

  I think on balance, it's probably a reasonable idea, but I haven't done a
detailed analysis of the risk so it's possible I've overlooked something
disastrous.  Since 1.7 is still experimental (albeit stabilising rapidly), I
guess we could even just go ahead, and revert it iff problems arise.

  CGF?  You asked a couple of questions and then dropped out of the thread for
a couple of days.  Have you reached any conclusions?

    cheers,
      DaveK


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix type inconsistencies in stdint.h

Eric Blake (cygwin)
In reply to this post by Christopher Faylor-8
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Christopher Faylor on 4/4/2009 12:24 AM:
>> Because our stdint.h types are divergent from Linux, and changing them
>> instead could cause yet another ABI break.
>
> Why would changing uint32_t from 'unsigned long' to 'unsigned int' break
> anything?  It looks to me like that is a disaster waiting to happen if
> we ever provide a 64-bit port.

If we ever provide a 64-bit port, then we are free to use #ifdef magic to
select a different underlying type on 64-bit compiles than on 32-bit
compiles.  In one sense, using a different type between the two builds
will flush out coding bugs where the wrong type specifiers are used (for
example, printf("%ld", (int32_t)val) should have been written
printf("%"PRI32d, (int32_t)val).

On the other hand, the fact that cygwin differs from Linux is already
flushing out these types of coding bugs.  Making the ABI change now (which
probably won't affect C apps, but will definitely affect any C++ code that
used uint32_t and friends in mangled names) will mean that cygwin no
longer trips true bugs in apps originally written on Linux by people not
aware of the issue.  It means easier porting jobs to cygwin, but also that
lurking bugs are that much harder to find when porting to yet another system.

I'm not sure we need the ABI change.  But I'm with Dave that IF we decide
the ABI change is the right thing to do, then NOW is the only time worth
doing it.

- --
Don't work too hard, make some time for fun as well!

Eric Blake             [hidden email]
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAknbTSwACgkQ84KuGfSFAYDP8ACgsENCESTjm6ANnyiBKPcTLr3E
zWcAniczYlqVaN5WiEH82riv3aKkZg9b
=YaqT
-----END PGP SIGNATURE-----
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix type inconsistencies in stdint.h

Charles Wilson-2
Eric Blake wrote:
> Making the ABI change now (which
> probably won't affect C apps, but will definitely affect any C++ code that
> used uint32_t and friends in mangled names)
>
> But I'm with Dave that IF we decide
> the ABI change is the right thing to do, then NOW is the only time worth
> doing it.

Especially as the transition to
gcc4/dw2-eh/shared-libgcc/shared-libstdc++/--enable-fully-dynamic-string
is *definitely* an ABI break for C++, anyway.

--
Chuck

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix type inconsistencies in stdint.h

Corinna Vinschen-2
On Apr  7 09:06, Charles Wilson wrote:

> Eric Blake wrote:
> > Making the ABI change now (which
> > probably won't affect C apps, but will definitely affect any C++ code that
> > used uint32_t and friends in mangled names)
> >
> > But I'm with Dave that IF we decide
> > the ABI change is the right thing to do, then NOW is the only time worth
> > doing it.
>
> Especially as the transition to
> gcc4/dw2-eh/shared-libgcc/shared-libstdc++/--enable-fully-dynamic-string
> is *definitely* an ABI break for C++, anyway.

Good point, I guess.  So, if we all agree on that, I'd suggest to
change Dave's patch to the one below.


Corinna


        * include/stdint.h (int_least32_t): Define as int.
        (uint_least32_t): Ditto, unsigned.
        (int_fast16_t): Define as int.
        (int_fast32_t): Ditto.
        (uint_fast16_t): Ditto, unsigned.
        (uint_fast32_t): Ditto.
        (UINT32_MAX): Remove `L' long marker.
        (UINT_LEAST32_MAX): Ditto.
        (UINT_FAST16_MAX): Ditto.
        (UINT_FAST32_MAX): Ditto.
        (INT32_C): Ditto.
        (UINT32_C): Ditto.


Index: include/stdint.h
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/include/stdint.h,v
retrieving revision 1.10
diff -u -p -r1.10 stdint.h
--- include/stdint.h 17 May 2008 21:34:05 -0000 1.10
+++ include/stdint.h 7 Apr 2009 13:12:48 -0000
@@ -33,24 +33,24 @@ typedef unsigned long long uint64_t;
 
 typedef signed char int_least8_t;
 typedef short int_least16_t;
-typedef long int_least32_t;
+typedef int int_least32_t;
 typedef long long int_least64_t;
 
 typedef unsigned char uint_least8_t;
 typedef unsigned short uint_least16_t;
-typedef unsigned long uint_least32_t;
+typedef unsigned int uint_least32_t;
 typedef unsigned long long uint_least64_t;
 
 /* Fastest minimum-width integer types */
 
 typedef signed char int_fast8_t;
-typedef long int_fast16_t;
-typedef long int_fast32_t;
+typedef int int_fast16_t;
+typedef int int_fast32_t;
 typedef long long int_fast64_t;
 
 typedef unsigned char uint_fast8_t;
-typedef unsigned long uint_fast16_t;
-typedef unsigned long uint_fast32_t;
+typedef unsigned int uint_fast16_t;
+typedef unsigned int uint_fast32_t;
 typedef unsigned long long uint_fast64_t;
 
 /* Integer types capable of holding object pointers */
@@ -80,7 +80,7 @@ typedef unsigned long long uintmax_t;
 
 #define UINT8_MAX (255)
 #define UINT16_MAX (65535)
-#define UINT32_MAX (4294967295UL)
+#define UINT32_MAX (4294967295U)
 #define UINT64_MAX (18446744073709551615ULL)
 
 /* Limits of minimum-width integer types */
@@ -97,7 +97,7 @@ typedef unsigned long long uintmax_t;
 
 #define UINT_LEAST8_MAX (255)
 #define UINT_LEAST16_MAX (65535)
-#define UINT_LEAST32_MAX (4294967295UL)
+#define UINT_LEAST32_MAX (4294967295U)
 #define UINT_LEAST64_MAX (18446744073709551615ULL)
 
 /* Limits of fastest minimum-width integer types */
@@ -113,8 +113,8 @@ typedef unsigned long long uintmax_t;
 #define INT_FAST64_MAX (9223372036854775807LL)
 
 #define UINT_FAST8_MAX (255)
-#define UINT_FAST16_MAX (4294967295UL)
-#define UINT_FAST32_MAX (4294967295UL)
+#define UINT_FAST16_MAX (4294967295U)
+#define UINT_FAST32_MAX (4294967295U)
 #define UINT_FAST64_MAX (18446744073709551615ULL)
 
 /* Limits of integer types capable of holding object pointers */
@@ -166,12 +166,12 @@ typedef unsigned long long uintmax_t;
 
 #define INT8_C(x) x
 #define INT16_C(x) x
-#define INT32_C(x) x ## L
+#define INT32_C(x) x
 #define INT64_C(x) x ## LL
 
 #define UINT8_C(x) x
 #define UINT16_C(x) x
-#define UINT32_C(x) x ## UL
+#define UINT32_C(x) x ## U
 #define UINT64_C(x) x ## ULL
 
 /* Macros for greatest-width integer constant expressions */

--
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix type inconsistencies in stdint.h

Dave Korn-6
In reply to this post by Eric Blake (cygwin)
Eric Blake wrote:

> According to Christopher Faylor on 4/4/2009 12:24 AM:
>>> Because our stdint.h types are divergent from Linux, and changing them
>>> instead could cause yet another ABI break.
>> Why would changing uint32_t from 'unsigned long' to 'unsigned int' break
>> anything?  It looks to me like that is a disaster waiting to happen if
>> we ever provide a 64-bit port.
>
> If we ever provide a 64-bit port, then we are free to use #ifdef magic to
> select a different underlying type on 64-bit compiles than on 32-bit
> compiles.  

  Indeed, that would be more linux like, because that's how linux does it, e.g
(from http://linux.die.net/include/stdint.h):

# if __WORDSIZE == 64
typedef long int int64_t;
# else
__extension__
typedef long long int int64_t;
# endif

    cheers,
      DaveK


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix type inconsistencies in stdint.h

Christopher Faylor-8
In reply to this post by Dave Korn-6
On Sat, Apr 04, 2009 at 08:23:49AM +0100, Dave Korn wrote:

>Christopher Faylor wrote:
>
>  Ah, I could address a bit more to these two questions as well:
>
>> Isn't a long 32 bits?  What would be the ABI breakage in changing that
>> one typedef rather than lots of #defines?  
>
>  Yes, a long is 32 bits, but while that makes for binary ABI
>(calling-convention) compatibility it isn't the same thing in the C and C++
>types system.  Therefore the underlying types are an inextricably woven part
>of the overall C-language ABI as well as their physical bit sizes.  Changing
>them certainly has the potential to change the ABI, particularly in C++, but I
>think it also might potentially render some of the compiler's aliasing
>assumptions invalid when linking code using the new definitions against
>objects or libraries using the old.
>
>Changing the limits #defines, OTOH, is absolutely guaranteed ABI
>neutral.  They really are "just constants" at runtime, and constants
>don't get mangled or alias anything.  So I reckon it's a safer way to
>proceed and I don't yet see any potential 64-bit problems down the line
>if we leave everything as it currently stands.
>
>Can you see anything I've overlooked in this analysis?

I don't entirely understand when people think it's ok to make sweeping
changes for 1.7 and when they think we need to be conservative.

I think it is very regrettable that Cygwin doesn't have the same int
types as linux and it would be interesting to see how much would be
broken by changing these types.

cgf
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix type inconsistencies in stdint.h

Christopher Faylor-8
In reply to this post by Eric Blake (cygwin)
On Tue, Apr 07, 2009 at 06:56:53AM -0600, Eric Blake wrote:

>According to Christopher Faylor on 4/4/2009 12:24 AM:
>>> Because our stdint.h types are divergent from Linux, and changing them
>>> instead could cause yet another ABI break.
>>
>>Why would changing uint32_t from 'unsigned long' to 'unsigned int'
>>break anything?  It looks to me like that is a disaster waiting to
>>happen if we ever provide a 64-bit port.
>
>If we ever provide a 64-bit port, then we are free to use #ifdef magic
>to select a different underlying type on 64-bit compiles than on 32-bit
>compiles.  In one sense, using a different type between the two builds
>will flush out coding bugs where the wrong type specifiers are used
>(for example, printf("%ld", (int32_t)val) should have been written
>printf("%"PRI32d, (int32_t)val).

Yes, of course, you could #ifdef these things.  If flushing out coding
bugs was a core requirement for Cygwin that would be an interesting
thing to do.  However our goal it really is quite the contrary.

We're suposed to make things easy to move between systems, warts and
all.  The goal is not to provide us with the opportunity to wag our
finger at hapless users in the Cygwin mailing who barely understand how
to run configure and make but nevertheless have a program that compiles
on linux and breaks on cygwin.  We're supposed to make things easier.

If people want to sanity check their code, they can buy a Plum Hall
or something.

cgf
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix type inconsistencies in stdint.h

Christopher Faylor-8
In reply to this post by Dave Korn-6
On Tue, Apr 07, 2009 at 01:33:24PM +0100, Dave Korn wrote:
>CGF?  You asked a couple of questions and then dropped out of the
>thread for a couple of days.  Have you reached any conclusions?

Stomach flu will do that to you...

cgf
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix type inconsistencies in stdint.h

Charles Wilson-2
In reply to this post by Christopher Faylor-8
Christopher Faylor wrote:
> I don't entirely understand when people think it's ok to make sweeping
> changes for 1.7 and when they think we need to be conservative.

MHO is that 1.7+gcc4 is already such a sweeping change (e.g.
"conservative" left the building sometime last year), that if we DO plan
on any more such sweeping changes before cygwin2.dll it's better to do
'em now.

OTOH, if we DON'T actually plan on any more such changes, then there's
no reason to make changes gratuitously, no matter how Just Mean We Are.

> I think it is very regrettable that Cygwin doesn't have the same int
> types as linux and it would be interesting to see how much would be
> broken by changing these types.

"Interesting" in the sense of the old Chinese curse [*], I assume?

--
Chuck

[*] "May you live in interesting times"

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix type inconsistencies in stdint.h

Christopher Faylor-8
On Tue, Apr 07, 2009 at 10:57:02AM -0400, Charles Wilson wrote:

>Christopher Faylor wrote:
>> I don't entirely understand when people think it's ok to make sweeping
>> changes for 1.7 and when they think we need to be conservative.
>
>MHO is that 1.7+gcc4 is already such a sweeping change (e.g.
>"conservative" left the building sometime last year), that if we DO plan
>on any more such sweeping changes before cygwin2.dll it's better to do
>'em now.
>
>OTOH, if we DON'T actually plan on any more such changes, then there's
>no reason to make changes gratuitously, no matter how Just Mean We Are.
>
>> I think it is very regrettable that Cygwin doesn't have the same int
>> types as linux and it would be interesting to see how much would be
>> broken by changing these types.
>
>"Interesting" in the sense of the old Chinese curse [*], I assume?

Or as in the "WJM" aforementioned sense.

cgf
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix type inconsistencies in stdint.h

Corinna Vinschen-2
On Apr  7 11:08, Christopher Faylor wrote:

> On Tue, Apr 07, 2009 at 10:57:02AM -0400, Charles Wilson wrote:
> >Christopher Faylor wrote:
> >> I don't entirely understand when people think it's ok to make sweeping
> >> changes for 1.7 and when they think we need to be conservative.
> >
> >MHO is that 1.7+gcc4 is already such a sweeping change (e.g.
> >"conservative" left the building sometime last year), that if we DO plan
> >on any more such sweeping changes before cygwin2.dll it's better to do
> >'em now.
> >
> >OTOH, if we DON'T actually plan on any more such changes, then there's
> >no reason to make changes gratuitously, no matter how Just Mean We Are.
> >
> >> I think it is very regrettable that Cygwin doesn't have the same int
> >> types as linux and it would be interesting to see how much would be
> >> broken by changing these types.
> >
> >"Interesting" in the sense of the old Chinese curse [*], I assume?
>
> Or as in the "WJM" aforementioned sense.

Sounds like "WJM" matches exactly what my patch does.  I've checked it in.


Corinna

--
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat
12