| Message ID | 47C91597-B6B1-4351-9DC3-30E405EC480B@sunshineco.com |
|---|---|
| State | New |
| Headers | show |
Am 01.11.2011 09:09, schrieb Eric Sunshine: > Perhaps the following alternative solution would be more palatable? It's > still tremendously ugly, but is localized to cocoa.m, thus less intrusive. > > -- >8 -- > Subject: [PATCH] softfloat: Avoid uint16 type conflict on Darwin > > cocoa.m includes <Security/cssmconfig.h> indirectly via <Cocoa/Cocoa.h>. > cssmconfig.h defines type uint16 which unfortunately conflicts with the > definition in qemu's softfloat.h, thus resulting in compilation failure. > To work around the problem, #define _UINT16, which informs cssmconfig.h > that uint16 is already defined and that it should not apply its own > definition. Thanks for the suggestion! _UINT16 is an interesting suggestion, however softfloat's uint16 is not uint16_t but int, so I'd rather not do it that way around. (I had also decided against the AIX path of never defining uint16 and always using system definitions, since that wouldn't work outside Cocoa code.) Do you have any thoughts about the include path issue? If we could keep QEMU code from getting into #import <Cocoa/Cocoa.h> then we could redefine the system type instead, in cocoa.m. Andreas
On Nov 1, 2011, at 12:37 PM, Andreas Färber wrote: > Am 01.11.2011 09:09, schrieb Eric Sunshine: >> Perhaps the following alternative solution would be more palatable? >> It's >> still tremendously ugly, but is localized to cocoa.m, thus less >> intrusive. >> >> -- >8 -- >> Subject: [PATCH] softfloat: Avoid uint16 type conflict on Darwin >> >> cocoa.m includes <Security/cssmconfig.h> indirectly via <Cocoa/ >> Cocoa.h>. >> cssmconfig.h defines type uint16 which unfortunately conflicts with >> the >> definition in qemu's softfloat.h, thus resulting in compilation >> failure. >> To work around the problem, #define _UINT16, which informs >> cssmconfig.h >> that uint16 is already defined and that it should not apply its own >> definition. > > Thanks for the suggestion! _UINT16 is an interesting suggestion, > however > softfloat's uint16 is not uint16_t but int, so I'd rather not do it > that > way around. > > (I had also decided against the AIX path of never defining uint16 and > always using system definitions, since that wouldn't work outside > Cocoa > code.) > > Do you have any thoughts about the include path issue? If we could > keep > QEMU code from getting into #import <Cocoa/Cocoa.h> then we could > redefine the system type instead, in cocoa.m. Is the intention to trust uint16 from <Security/cssmconfig.h> over the one softfloat.h? If so, shouldn't we be taking as many type definitions from <Security/cssmconfig.h> as we can rather than just this one? (I'm not recommending it; just trying to understand the goal.) -- ES
Am 01.11.2011 19:47, schrieb Eric Sunshine: > On Nov 1, 2011, at 12:37 PM, Andreas Färber wrote: >> Am 01.11.2011 09:09, schrieb Eric Sunshine: >>> Perhaps the following alternative solution would be more palatable? It's >>> still tremendously ugly, but is localized to cocoa.m, thus less >>> intrusive. >>> >>> -- >8 -- >>> Subject: [PATCH] softfloat: Avoid uint16 type conflict on Darwin >>> >>> cocoa.m includes <Security/cssmconfig.h> indirectly via <Cocoa/Cocoa.h>. >>> cssmconfig.h defines type uint16 which unfortunately conflicts with the >>> definition in qemu's softfloat.h, thus resulting in compilation failure. >>> To work around the problem, #define _UINT16, which informs cssmconfig.h >>> that uint16 is already defined and that it should not apply its own >>> definition. >> >> Thanks for the suggestion! _UINT16 is an interesting suggestion, however >> softfloat's uint16 is not uint16_t but int, so I'd rather not do it that >> way around. >> >> (I had also decided against the AIX path of never defining uint16 and >> always using system definitions, since that wouldn't work outside Cocoa >> code.) >> >> Do you have any thoughts about the include path issue? If we could keep >> QEMU code from getting into #import <Cocoa/Cocoa.h> then we could >> redefine the system type instead, in cocoa.m. > > Is the intention to trust uint16 from <Security/cssmconfig.h> over the > one softfloat.h? If so, shouldn't we be taking as many type definitions > from <Security/cssmconfig.h> as we can rather than just this one? (I'm > not recommending it; just trying to understand the goal.) Short-term goal: make Darwin build 1.0 without breaking others Long-term goal: not use uint16 etc. in QEMU at all Don't see what you mean with "taking as many type definitions". After uint16 I get no further conflicts for --enable-system --disable-user, so what is there to take? Andreas
On Nov 1, 2011, at 2:52 PM, Andreas Färber wrote: > Am 01.11.2011 19:47, schrieb Eric Sunshine: >> On Nov 1, 2011, at 12:37 PM, Andreas Färber wrote: >>> Am 01.11.2011 09:09, schrieb Eric Sunshine: >>>> Perhaps the following alternative solution would be more >>>> palatable? It's >>>> still tremendously ugly, but is localized to cocoa.m, thus less >>>> intrusive. >>>> >>>> -- >8 -- >>>> Subject: [PATCH] softfloat: Avoid uint16 type conflict on Darwin >>>> >>>> cocoa.m includes <Security/cssmconfig.h> indirectly via <Cocoa/ >>>> Cocoa.h>. >>>> cssmconfig.h defines type uint16 which unfortunately conflicts >>>> with the >>>> definition in qemu's softfloat.h, thus resulting in compilation >>>> failure. >>>> To work around the problem, #define _UINT16, which informs >>>> cssmconfig.h >>>> that uint16 is already defined and that it should not apply its own >>>> definition. >>> >>> Thanks for the suggestion! _UINT16 is an interesting suggestion, >>> however >>> softfloat's uint16 is not uint16_t but int, so I'd rather not do >>> it that >>> way around. >>> >>> (I had also decided against the AIX path of never defining uint16 >>> and >>> always using system definitions, since that wouldn't work outside >>> Cocoa >>> code.) >>> >>> Do you have any thoughts about the include path issue? If we could >>> keep >>> QEMU code from getting into #import <Cocoa/Cocoa.h> then we could >>> redefine the system type instead, in cocoa.m. >> >> Is the intention to trust uint16 from <Security/cssmconfig.h> over >> the >> one softfloat.h? If so, shouldn't we be taking as many type >> definitions >> from <Security/cssmconfig.h> as we can rather than just this one? >> (I'm >> not recommending it; just trying to understand the goal.) > > Short-term goal: make Darwin build 1.0 without breaking others > Long-term goal: not use uint16 etc. in QEMU at all > > Don't see what you mean with "taking as many type definitions". After > uint16 I get no further conflicts for --enable-system --disable-user, > so what is there to take? Sorry for not being clear. My question was not about build errors but about semantics. What I meant was that, with this patch, you are giving special preference only to Darwin's definition of uint16, but then contrarily preferring softfloat's definition of int16. Likewise, softfloat's uint32, int32, uint64, int64 from softfloat are trusted over the definitions from Darwin. Other than the fact that only uint16 led to a compilation error, it does not make sense semantically to single out Darwin's definition of only this one type. I would think that we should be trusting either _all_ Darwin type definitions or _none_. Singling out just this one seems anomalous. -- ES
Am 01.11.2011 20:06, schrieb Eric Sunshine: > > On Nov 1, 2011, at 2:52 PM, Andreas Färber wrote: > >> Am 01.11.2011 19:47, schrieb Eric Sunshine: >>> On Nov 1, 2011, at 12:37 PM, Andreas Färber wrote: >>>> Am 01.11.2011 09:09, schrieb Eric Sunshine: >>>>> Perhaps the following alternative solution would be more palatable? >>>>> It's >>>>> still tremendously ugly, but is localized to cocoa.m, thus less >>>>> intrusive. >>>>> >>>>> -- >8 -- >>>>> Subject: [PATCH] softfloat: Avoid uint16 type conflict on Darwin >>>>> >>>>> cocoa.m includes <Security/cssmconfig.h> indirectly via >>>>> <Cocoa/Cocoa.h>. >>>>> cssmconfig.h defines type uint16 which unfortunately conflicts with >>>>> the >>>>> definition in qemu's softfloat.h, thus resulting in compilation >>>>> failure. >>>>> To work around the problem, #define _UINT16, which informs >>>>> cssmconfig.h >>>>> that uint16 is already defined and that it should not apply its own >>>>> definition. >>>> >>>> Thanks for the suggestion! _UINT16 is an interesting suggestion, >>>> however >>>> softfloat's uint16 is not uint16_t but int, so I'd rather not do it >>>> that >>>> way around. >>>> >>>> (I had also decided against the AIX path of never defining uint16 and >>>> always using system definitions, since that wouldn't work outside Cocoa >>>> code.) >>>> >>>> Do you have any thoughts about the include path issue? If we could keep >>>> QEMU code from getting into #import <Cocoa/Cocoa.h> then we could >>>> redefine the system type instead, in cocoa.m. >>> >>> Is the intention to trust uint16 from <Security/cssmconfig.h> over the >>> one softfloat.h? If so, shouldn't we be taking as many type definitions >>> from <Security/cssmconfig.h> as we can rather than just this one? (I'm >>> not recommending it; just trying to understand the goal.) >> >> Short-term goal: make Darwin build 1.0 without breaking others >> Long-term goal: not use uint16 etc. in QEMU at all >> >> Don't see what you mean with "taking as many type definitions". After >> uint16 I get no further conflicts for --enable-system --disable-user, >> so what is there to take? > > Sorry for not being clear. My question was not about build errors but > about semantics. What I meant was that, with this patch, you are giving > special preference only to Darwin's definition of uint16, but then > contrarily preferring softfloat's definition of int16. Likewise, > softfloat's uint32, int32, uint64, int64 from softfloat are trusted over > the definitions from Darwin. > > Other than the fact that only uint16 led to a compilation error, it does > not make sense semantically to single out Darwin's definition of only > this one type. I would think that we should be trusting either _all_ > Darwin type definitions or _none_. Singling out just this one seems > anomalous. Listen, I dont have time for this. We have three options: 1) I can say, "I'm the Cocoa maintainer for multiple years now, I don't care if someone pops up day before the deadline and complains" and just push my version of preference. 2) We disagree on the solution, so I'm fair and send a pull request for the three other non-controversial patches only and 1.0 remains broken on Darwin. 3) You send a patch based on this one, detailing what additional changes you suggest and we'll see clearer what exactly you mean. I'm not preferring any definition of int16, uint32, etc., there simply is no conflict, so why would I clutter softfloat.h with unnecessary workarounds that we want to go away anyway. Feel free to refactor fpu/* instead to not use uint16 in the first place. I did so once and it was rejected, so I'm not too inclined to do that again unless we decide on how exactly to proceed with that! Andreas
On Nov 1, 2011, at 3:06 PM, Eric Sunshine wrote: > On Nov 1, 2011, at 2:52 PM, Andreas Färber wrote: > >> Am 01.11.2011 19:47, schrieb Eric Sunshine: >>> On Nov 1, 2011, at 12:37 PM, Andreas Färber wrote: >>>> Am 01.11.2011 09:09, schrieb Eric Sunshine: >>>>> Perhaps the following alternative solution would be more >>>>> palatable? It's >>>>> still tremendously ugly, but is localized to cocoa.m, thus less >>>>> intrusive. >>>>> >>>>> -- >8 -- >>>>> Subject: [PATCH] softfloat: Avoid uint16 type conflict on Darwin >>>>> >>>>> cocoa.m includes <Security/cssmconfig.h> indirectly via <Cocoa/ >>>>> Cocoa.h>. >>>>> cssmconfig.h defines type uint16 which unfortunately conflicts >>>>> with the >>>>> definition in qemu's softfloat.h, thus resulting in compilation >>>>> failure. >>>>> To work around the problem, #define _UINT16, which informs >>>>> cssmconfig.h >>>>> that uint16 is already defined and that it should not apply its >>>>> own >>>>> definition. >>>> >>>> Thanks for the suggestion! _UINT16 is an interesting suggestion, >>>> however >>>> softfloat's uint16 is not uint16_t but int, so I'd rather not do >>>> it that >>>> way around. >>>> >>>> (I had also decided against the AIX path of never defining uint16 >>>> and >>>> always using system definitions, since that wouldn't work outside >>>> Cocoa >>>> code.) >>>> >>>> Do you have any thoughts about the include path issue? If we >>>> could keep >>>> QEMU code from getting into #import <Cocoa/Cocoa.h> then we could >>>> redefine the system type instead, in cocoa.m. >>> >>> Is the intention to trust uint16 from <Security/cssmconfig.h> over >>> the >>> one softfloat.h? If so, shouldn't we be taking as many type >>> definitions >>> from <Security/cssmconfig.h> as we can rather than just this one? >>> (I'm >>> not recommending it; just trying to understand the goal.) >> >> Short-term goal: make Darwin build 1.0 without breaking others >> Long-term goal: not use uint16 etc. in QEMU at all >> >> Don't see what you mean with "taking as many type definitions". After >> uint16 I get no further conflicts for --enable-system --disable-user, >> so what is there to take? > > Sorry for not being clear. My question was not about build errors > but about semantics. What I meant was that, with this patch, you are > giving special preference only to Darwin's definition of uint16, but > then contrarily preferring softfloat's definition of int16. > Likewise, softfloat's uint32, int32, uint64, int64 from softfloat > are trusted over the definitions from Darwin. > > Other than the fact that only uint16 led to a compilation error, it > does not make sense semantically to single out Darwin's definition > of only this one type. I would think that we should be trusting > either _all_ Darwin type definitions or _none_. Singling out just > this one seems anomalous. > > -- ES I forgot to mention that with your patch, only cocoa.m is seeing Darwin's definition of uint16. The rest of qemu is seeing the definition from softfloat.h. This inconsistency hopefully is not harmful in the short-term, which is why I asked about the goal. If the short-term idea is for cocoa.m to build cleanly but not to worry much that cocoa.m sees a different uint16 from the rest of qemu, then the less intrusive patch involving only cocoa.m may be preferable. -- ES
Am 01.11.2011 20:25, schrieb Eric Sunshine: > I forgot to mention that with your patch, only cocoa.m is seeing > Darwin's definition of uint16. The rest of qemu is seeing the definition > from softfloat.h. This inconsistency hopefully is not harmful in the > short-term, which is why I asked about the goal. If the short-term idea > is for cocoa.m to build cleanly but not to worry much that cocoa.m sees > a different uint16 from the rest of qemu, then the less intrusive patch > involving only cocoa.m may be preferable. Ouch. Meaning both our softfloat patches are wrong, so I'll go with (2). It's evening in Italy and the last day for submaintainer PULLs. AF
On 1 November 2011 19:25, Andreas Färber <andreas.faerber@web.de> wrote: > Feel free to refactor fpu/* instead to not use uint16 in the first > place. I did so once and it was rejected, so I'm not too inclined to do > that again unless we decide on how exactly to proceed with that! I think we could probably resolve that with a little bit of benchmarking about whether it really makes any difference whether we use an int16_t or an int32_t for the variables where softfloat has int16 currently; but benchmarking is tedious so I've never got round to it :-) -- PMM
On Nov 1, 2011, at 3:25 PM, Andreas Färber wrote: > Am 01.11.2011 20:06, schrieb Eric Sunshine: >> >> On Nov 1, 2011, at 2:52 PM, Andreas Färber wrote: >> >>> Am 01.11.2011 19:47, schrieb Eric Sunshine: >>>> On Nov 1, 2011, at 12:37 PM, Andreas Färber wrote: >>>>> Am 01.11.2011 09:09, schrieb Eric Sunshine: >>>>>> Perhaps the following alternative solution would be more >>>>>> palatable? >>>>>> It's >>>>>> still tremendously ugly, but is localized to cocoa.m, thus less >>>>>> intrusive. >>>>>> >>>>>> -- >8 -- >>>>>> Subject: [PATCH] softfloat: Avoid uint16 type conflict on Darwin >>>>>> >>>>>> cocoa.m includes <Security/cssmconfig.h> indirectly via >>>>>> <Cocoa/Cocoa.h>. >>>>>> cssmconfig.h defines type uint16 which unfortunately conflicts >>>>>> with >>>>>> the >>>>>> definition in qemu's softfloat.h, thus resulting in compilation >>>>>> failure. >>>>>> To work around the problem, #define _UINT16, which informs >>>>>> cssmconfig.h >>>>>> that uint16 is already defined and that it should not apply its >>>>>> own >>>>>> definition. >>>>> >>>>> Thanks for the suggestion! _UINT16 is an interesting suggestion, >>>>> however >>>>> softfloat's uint16 is not uint16_t but int, so I'd rather not do >>>>> it >>>>> that >>>>> way around. >>>>> >>>>> (I had also decided against the AIX path of never defining >>>>> uint16 and >>>>> always using system definitions, since that wouldn't work >>>>> outside Cocoa >>>>> code.) >>>>> >>>>> Do you have any thoughts about the include path issue? If we >>>>> could keep >>>>> QEMU code from getting into #import <Cocoa/Cocoa.h> then we could >>>>> redefine the system type instead, in cocoa.m. >>>> >>>> Is the intention to trust uint16 from <Security/cssmconfig.h> >>>> over the >>>> one softfloat.h? If so, shouldn't we be taking as many type >>>> definitions >>>> from <Security/cssmconfig.h> as we can rather than just this one? >>>> (I'm >>>> not recommending it; just trying to understand the goal.) >>> >>> Short-term goal: make Darwin build 1.0 without breaking others >>> Long-term goal: not use uint16 etc. in QEMU at all >>> >>> Don't see what you mean with "taking as many type definitions". >>> After >>> uint16 I get no further conflicts for --enable-system --disable- >>> user, >>> so what is there to take? >> >> Sorry for not being clear. My question was not about build errors but >> about semantics. What I meant was that, with this patch, you are >> giving >> special preference only to Darwin's definition of uint16, but then >> contrarily preferring softfloat's definition of int16. Likewise, >> softfloat's uint32, int32, uint64, int64 from softfloat are trusted >> over >> the definitions from Darwin. >> >> Other than the fact that only uint16 led to a compilation error, it >> does >> not make sense semantically to single out Darwin's definition of only >> this one type. I would think that we should be trusting either _all_ >> Darwin type definitions or _none_. Singling out just this one seems >> anomalous. > > Listen, I dont have time for this. We have three options: > > 1) I can say, "I'm the Cocoa maintainer for multiple years now, I > don't > care if someone pops up day before the deadline and complains" and > just > push my version of preference. I hope that you do not interpret my alternate patch as a "complaint before the deadline". My intention only was to be helpful when I saw Peter's response [1], and thought that a less intrusive patch might be more acceptable. [1]: http://lists.nongnu.org/archive/html/qemu-devel/2011-10/msg03936.html -- ES
Am 01.11.2011 20:45, schrieb Eric Sunshine: > On Nov 1, 2011, at 3:25 PM, Andreas Färber wrote: >> Am 01.11.2011 20:06, schrieb Eric Sunshine: >>> >>> On Nov 1, 2011, at 2:52 PM, Andreas Färber wrote: >>> >>>> Am 01.11.2011 19:47, schrieb Eric Sunshine: >>>>> On Nov 1, 2011, at 12:37 PM, Andreas Färber wrote: >>>>>> Am 01.11.2011 09:09, schrieb Eric Sunshine: >>>>>>> Perhaps the following alternative solution would be more palatable? >>>>>>> It's >>>>>>> still tremendously ugly, but is localized to cocoa.m, thus less >>>>>>> intrusive. >>>>>>> >>>>>>> -- >8 -- >>>>>>> Subject: [PATCH] softfloat: Avoid uint16 type conflict on Darwin >>>>>>> >>>>>>> cocoa.m includes <Security/cssmconfig.h> indirectly via >>>>>>> <Cocoa/Cocoa.h>. >>>>>>> cssmconfig.h defines type uint16 which unfortunately conflicts with >>>>>>> the >>>>>>> definition in qemu's softfloat.h, thus resulting in compilation >>>>>>> failure. >>>>>>> To work around the problem, #define _UINT16, which informs >>>>>>> cssmconfig.h >>>>>>> that uint16 is already defined and that it should not apply its own >>>>>>> definition. >>>>>> >>>>>> Thanks for the suggestion! _UINT16 is an interesting suggestion, >>>>>> however >>>>>> softfloat's uint16 is not uint16_t but int, so I'd rather not do it >>>>>> that >>>>>> way around. >>>>>> >>>>>> (I had also decided against the AIX path of never defining uint16 and >>>>>> always using system definitions, since that wouldn't work outside >>>>>> Cocoa >>>>>> code.) >>>>>> >>>>>> Do you have any thoughts about the include path issue? If we could >>>>>> keep >>>>>> QEMU code from getting into #import <Cocoa/Cocoa.h> then we could >>>>>> redefine the system type instead, in cocoa.m. >>>>> >>>>> Is the intention to trust uint16 from <Security/cssmconfig.h> over the >>>>> one softfloat.h? If so, shouldn't we be taking as many type >>>>> definitions >>>>> from <Security/cssmconfig.h> as we can rather than just this one? (I'm >>>>> not recommending it; just trying to understand the goal.) >>>> >>>> Short-term goal: make Darwin build 1.0 without breaking others >>>> Long-term goal: not use uint16 etc. in QEMU at all >>>> >>>> Don't see what you mean with "taking as many type definitions". After >>>> uint16 I get no further conflicts for --enable-system --disable-user, >>>> so what is there to take? >>> >>> Sorry for not being clear. My question was not about build errors but >>> about semantics. What I meant was that, with this patch, you are giving >>> special preference only to Darwin's definition of uint16, but then >>> contrarily preferring softfloat's definition of int16. Likewise, >>> softfloat's uint32, int32, uint64, int64 from softfloat are trusted over >>> the definitions from Darwin. >>> >>> Other than the fact that only uint16 led to a compilation error, it does >>> not make sense semantically to single out Darwin's definition of only >>> this one type. I would think that we should be trusting either _all_ >>> Darwin type definitions or _none_. Singling out just this one seems >>> anomalous. >> >> Listen, I dont have time for this. We have three options: >> >> 1) I can say, "I'm the Cocoa maintainer for multiple years now, I don't >> care if someone pops up day before the deadline and complains" and just >> push my version of preference. > > I hope that you do not interpret my alternate patch as a "complaint > before the deadline". Not your patch, I thanked you for it, but the seemingly nonconstructive complaints about my follow-up. I would've much preferred code. :/ > My intention only was to be helpful when I saw > Peter's response [1], and thought that a less intrusive patch might be > more acceptable. > > [1]: http://lists.nongnu.org/archive/html/qemu-devel/2011-10/msg03936.html Then it's not the intention we differ in, I had tried some solutions inside ui/cocoa.m myself before. Apart from non-intrusiveness further criteria are reversibility of the short-term fix[1] and ABI safety. I'll happily review new patches from tomorrow on. Regards, Andreas [1] We have similar lurking issues with [u]int* on Haiku/BeOS.
diff --git a/ui/cocoa.m b/ui/cocoa.m index d9e4e3d..ac15418 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -22,13 +22,14 @@ * THE SOFTWARE. */ -#import <Cocoa/Cocoa.h> -#include <crt_externs.h> - #include "qemu-common.h" #include "console.h" #include "sysemu.h" +#define _UINT16 +#import <Cocoa/Cocoa.h> +#include <crt_externs.h> + #ifndef MAC_OS_X_VERSION_10_4 #define MAC_OS_X_VERSION_10_4 1040 #endif