Re: [Qemu-devel] Patch: dyngen-exec.h for OpenBSD
Paul Brook wrote: On Thursday 05 April 2007 23:12, Todd T. Fries wrote: Penned by Thiemo Seufer on 20070402 10:54.53, we have: | > /* NOTE: standard headers should be used with special care at this | > point because host CPU registers are used as global variables. Some | > host headers do not allow that. */ | > #include | > - | > +#ifdef __OpenBSD__ | > +#include Hello? Portability? sys/types.h defines these types portably. Doing so the way this code does it, is not portable. If you want portability you should be including stdint.h (or inttypes.h for old, broken systems). I thought I'd add that this isn't just portability. stdint.h is what C99 mandates although as Paul mentions, some older systems used inttypes.h. Regards, Anthony Liguori Why is it that qemu knows what the definition of these prototypes are on all systems without consulting the header files. I have a better idea, lets let the header files define the prototypes. Who would have though of that? See the big NOTE: comment above. dyngen is inherently unportable. Paul
Re: [Qemu-devel] Patch: dyngen-exec.h for OpenBSD
On Thursday 05 April 2007 23:12, Todd T. Fries wrote: > Penned by Thiemo Seufer on 20070402 10:54.53, we have: > | > /* NOTE: standard headers should be used with special care at this > | > point because host CPU registers are used as global variables. Some > | > host headers do not allow that. */ > | > #include > | > - > | > +#ifdef __OpenBSD__ > | > +#include > Hello? Portability? sys/types.h defines these types portably. > Doing so the way this code does it, is not portable. If you want portability you should be including stdint.h (or inttypes.h for old, broken systems). > Why is it that qemu knows what the definition of these prototypes > are on all systems without consulting the header files. I have a > better idea, lets let the header files define the prototypes. > Who would have though of that? See the big NOTE: comment above. dyngen is inherently unportable. Paul
Re: [Qemu-devel] Patch: dyngen-exec.h for OpenBSD
Todd T. Fries wrote: > > Penned by Thiemo Seufer on 20070402 10:54.53, we have: > | > /* NOTE: standard headers should be used with special care at this > | > point because host CPU registers are used as global variables. Some > | > host headers do not allow that. */ > | > #include > | > - > | > +#ifdef __OpenBSD__ > | > +#include > | > +#else > | > typedef unsigned char uint8_t; > | > typedef unsigned short uint16_t; > | > typedef unsigned int uint32_t; > | > @@ -61,6 +65,7 @@ typedef signed long int64_t; > | > typedef signed long long int64_t; > | > #endif > | > #endif > | > +#endif > | > | Is this specialcase really needed for OpenBSD? > > Can you honestly tell me that on all platforms this is true? > > Hello? Portability? sys/types.h defines these types portably. > Doing so the way this code does it, is not portable. > > I left your non portable code to you; if systems in general > define these types in sys/types.h then just remove the > typedefs, problem solved. Please consider the NOTE: above those includes. > | > /* XXX: This may be wrong for 64-bit ILP32 hosts. */ > | > typedef void * host_reg_t; > | > @@ -78,11 +83,15 @@ typedef void * host_reg_t; > | > #define UINT32_MAX (4294967295U) > | > #define UINT64_MAX ((uint64_t)(18446744073709551615)) > | > > | > +#ifdef __OpenBSD__ > | > +typedef struct __sFILE FILE; > | > +#else > | > typedef struct FILE FILE; > | > extern int fprintf(FILE *, const char *, ...); > | > extern int printf(const char *, ...); > | > #undef NULL > | > #define NULL 0 > | > +#endif > | > | Shouldn't this cover only the FILE typedef? > > Why is it that qemu knows what the definition of these prototypes > are on all systems without consulting the header files. I have a > better idea, lets let the header files define the prototypes. > Who would have though of that? > > .. of course I purposefully intended to remove cruft that is > in header files and belongs in header files. See above. Thiemo
Re: [Qemu-devel] Patch: dyngen-exec.h for OpenBSD
Penned by Thiemo Seufer on 20070402 10:54.53, we have: | > /* NOTE: standard headers should be used with special care at this | > point because host CPU registers are used as global variables. Some | > host headers do not allow that. */ | > #include | > - | > +#ifdef __OpenBSD__ | > +#include | > +#else | > typedef unsigned char uint8_t; | > typedef unsigned short uint16_t; | > typedef unsigned int uint32_t; | > @@ -61,6 +65,7 @@ typedef signed long int64_t; | > typedef signed long long int64_t; | > #endif | > #endif | > +#endif | | Is this specialcase really needed for OpenBSD? Can you honestly tell me that on all platforms this is true? Hello? Portability? sys/types.h defines these types portably. Doing so the way this code does it, is not portable. I left your non portable code to you; if systems in general define these types in sys/types.h then just remove the typedefs, problem solved. | > /* XXX: This may be wrong for 64-bit ILP32 hosts. */ | > typedef void * host_reg_t; | > @@ -78,11 +83,15 @@ typedef void * host_reg_t; | > #define UINT32_MAX (4294967295U) | > #define UINT64_MAX ((uint64_t)(18446744073709551615)) | > | > +#ifdef __OpenBSD__ | > +typedef struct __sFILE FILE; | > +#else | > typedef struct FILE FILE; | > extern int fprintf(FILE *, const char *, ...); | > extern int printf(const char *, ...); | > #undef NULL | > #define NULL 0 | > +#endif | | Shouldn't this cover only the FILE typedef? Why is it that qemu knows what the definition of these prototypes are on all systems without consulting the header files. I have a better idea, lets let the header files define the prototypes. Who would have though of that? .. of course I purposefully intended to remove cruft that is in header files and belongs in header files. I'll look at your other comments later, but these I see later in the discussion are nearing inclusion with your recommended tweaks. I'll submit patches to correct things again if necessary. Thanks, -- Todd Fries .. [EMAIL PROTECTED] _ | \ 1.636.410.0632 (voice) | Free Daemon Consulting, LLC \ 1.405.227.9094 (voice) | http://FreeDaemonConsulting.com \ 1.866.792.3418 (FAX) | "..in support of free software solutions." \ 250797 (FWD) | \ \\ 37E7 D3EB 74D0 8D66 A68D B866 0326 204E 3F42 004A http://todd.fries.net/pgp.txt
Re: [Qemu-devel] Patch: dyngen-exec.h for OpenBSD
In message: <[EMAIL PROTECTED]> Thiemo Seufer <[EMAIL PROTECTED]> writes: : M. Warner Losh wrote: : > In message: <[EMAIL PROTECTED]> : > Thiemo Seufer <[EMAIL PROTECTED]> writes: : > : I made that "#ifdef _BSD" based on the assumption it is ok for all : > : BSD variants, including Darwin. : > : > _BSD isn't defined on all variants of BSD. sys/param.h defines BSD to : > be 199506 on all BSD systems (at least all of them derived from 4.4BSD : > lite). sys/param.h also defines BSD4_3 and BSD4_4. FreeBSD defines : > __FreeBSD__ in the compiler, NetBSD defined __NetBSD__, OpenBSD : > defines __OpenBSD__. I'm unsure what darwin/osx define. : > : > so unless I missed a change elsewhere in the build system to define : > _BSD, this change needs some more thought. : : It is already used in other files. The define in qemu comes from the : configure script via config.h, which might be a bug. Somehow I missed that detail... In that case, nevermind :-) Warner
Re: [Qemu-devel] Patch: dyngen-exec.h for OpenBSD
M. Warner Losh wrote: > In message: <[EMAIL PROTECTED]> > Thiemo Seufer <[EMAIL PROTECTED]> writes: > : I made that "#ifdef _BSD" based on the assumption it is ok for all > : BSD variants, including Darwin. > > _BSD isn't defined on all variants of BSD. sys/param.h defines BSD to > be 199506 on all BSD systems (at least all of them derived from 4.4BSD > lite). sys/param.h also defines BSD4_3 and BSD4_4. FreeBSD defines > __FreeBSD__ in the compiler, NetBSD defined __NetBSD__, OpenBSD > defines __OpenBSD__. I'm unsure what darwin/osx define. > > so unless I missed a change elsewhere in the build system to define > _BSD, this change needs some more thought. It is already used in other files. The define in qemu comes from the configure script via config.h, which might be a bug. Thiemo
Re: [Qemu-devel] Patch: dyngen-exec.h for OpenBSD
In message: <[EMAIL PROTECTED]> Thiemo Seufer <[EMAIL PROTECTED]> writes: : I made that "#ifdef _BSD" based on the assumption it is ok for all : BSD variants, including Darwin. _BSD isn't defined on all variants of BSD. sys/param.h defines BSD to be 199506 on all BSD systems (at least all of them derived from 4.4BSD lite). sys/param.h also defines BSD4_3 and BSD4_4. FreeBSD defines __FreeBSD__ in the compiler, NetBSD defined __NetBSD__, OpenBSD defines __OpenBSD__. I'm unsure what darwin/osx define. so unless I missed a change elsewhere in the build system to define _BSD, this change needs some more thought. Warner
Re: [Qemu-devel] Patch: dyngen-exec.h for OpenBSD
Juergen Keil wrote: > > > From: Thiemo Seufer <[EMAIL PROTECTED]> > > Todd T. Fries wrote: > > > This is relative to the 20070319 snapshot. > > > > > > > > > --- dyngen-exec.h.origMon Feb 5 17:01:54 2007 > > > +++ dyngen-exec.h Sat Mar 10 16:39:39 2007 > ... > > > /* XXX: This may be wrong for 64-bit ILP32 hosts. */ > > > typedef void * host_reg_t; > > > @@ -78,11 +83,15 @@ typedef void * host_reg_t; > > > #define UINT32_MAX (4294967295U) > > > #define UINT64_MAX ((uint64_t)(18446744073709551615)) > > > > > > +#ifdef __OpenBSD__ > > > +typedef struct __sFILE FILE; > > > +#else > > > typedef struct FILE FILE; > > > extern int fprintf(FILE *, const char *, ...); > > > extern int printf(const char *, ...); > > > #undef NULL > > > #define NULL 0 > > > +#endif > > > > Shouldn't this cover only the FILE typedef? > > Probably. > > My dyngen-exec.h has a similar change, when I made some NetBSD experiments: > > Index: dyngen-exec.h > === > RCS file: /cvsroot/qemu/qemu/dyngen-exec.h,v > retrieving revision 1.33 > diff -u -B -r1.33 dyngen-exec.h > --- dyngen-exec.h 30 Mar 2007 16:44:53 - 1.33 > +++ dyngen-exec.h 2 Apr 2007 09:42:03 - > @@ -78,7 +78,11 @@ > #define UINT32_MAX (4294967295U) > #define UINT64_MAX ((uint64_t)(18446744073709551615)) > > +#ifdef __NetBSD__ > +typedef struct __sFILE FILE; > +#else > typedef struct FILE FILE; > +#endif I made that "#ifdef _BSD" based on the assumption it is ok for all BSD variants, including Darwin. Thiemo
Re: [Qemu-devel] Patch: dyngen-exec.h for OpenBSD
> From: Thiemo Seufer <[EMAIL PROTECTED]> > Todd T. Fries wrote: > > This is relative to the 20070319 snapshot. > > > > > > --- dyngen-exec.h.orig Mon Feb 5 17:01:54 2007 > > +++ dyngen-exec.h Sat Mar 10 16:39:39 2007 ... > > /* XXX: This may be wrong for 64-bit ILP32 hosts. */ > > typedef void * host_reg_t; > > @@ -78,11 +83,15 @@ typedef void * host_reg_t; > > #define UINT32_MAX (4294967295U) > > #define UINT64_MAX ((uint64_t)(18446744073709551615)) > > > > +#ifdef __OpenBSD__ > > +typedef struct __sFILE FILE; > > +#else > > typedef struct FILE FILE; > > extern int fprintf(FILE *, const char *, ...); > > extern int printf(const char *, ...); > > #undef NULL > > #define NULL 0 > > +#endif > > Shouldn't this cover only the FILE typedef? Probably. My dyngen-exec.h has a similar change, when I made some NetBSD experiments: Index: dyngen-exec.h === RCS file: /cvsroot/qemu/qemu/dyngen-exec.h,v retrieving revision 1.33 diff -u -B -r1.33 dyngen-exec.h --- dyngen-exec.h 30 Mar 2007 16:44:53 - 1.33 +++ dyngen-exec.h 2 Apr 2007 09:42:03 - @@ -78,7 +78,11 @@ #define UINT32_MAX (4294967295U) #define UINT64_MAX ((uint64_t)(18446744073709551615)) +#ifdef __NetBSD__ +typedef struct __sFILE FILE; +#else typedef struct FILE FILE; +#endif extern int fprintf(FILE *, const char *, ...); extern int fputs(const char *, FILE *); extern int printf(const char *, ...);
Re: [Qemu-devel] Patch: dyngen-exec.h for OpenBSD
Todd T. Fries wrote: > This is relative to the 20070319 snapshot. > > > --- dyngen-exec.h.origMon Feb 5 17:01:54 2007 > +++ dyngen-exec.h Sat Mar 10 16:39:39 2007 > @@ -27,11 +27,15 @@ > #define _FILEDEFED > #endif > > +#include "config.h" > + Doesn't seem to be necessary in the header. > /* NOTE: standard headers should be used with special care at this > point because host CPU registers are used as global variables. Some > host headers do not allow that. */ > #include > - > +#ifdef __OpenBSD__ > +#include > +#else > typedef unsigned char uint8_t; > typedef unsigned short uint16_t; > typedef unsigned int uint32_t; > @@ -61,6 +65,7 @@ typedef signed long int64_t; > typedef signed long long int64_t; > #endif > #endif > +#endif Is this specialcase really needed for OpenBSD? > /* XXX: This may be wrong for 64-bit ILP32 hosts. */ > typedef void * host_reg_t; > @@ -78,11 +83,15 @@ typedef void * host_reg_t; > #define UINT32_MAX (4294967295U) > #define UINT64_MAX ((uint64_t)(18446744073709551615)) > > +#ifdef __OpenBSD__ > +typedef struct __sFILE FILE; > +#else > typedef struct FILE FILE; > extern int fprintf(FILE *, const char *, ...); > extern int printf(const char *, ...); > #undef NULL > #define NULL 0 > +#endif Shouldn't this cover only the FILE typedef? Thiemo
