Re: [Qemu-devel] [PATCH v2 2/4] softfloat: Avoid uint16 type conflict on Darwin

[Andreas Färber]

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.