(XS) RFR: 8077674: BSD build failures due to undefined macros
David Holmes
david.holmes at oracle.com
Mon Apr 20 07:36:19 UTC 2015
Thanks Dmitry.
Just realized I still need a (R)eviewer from runtime please. <sigh>
David
On 20/04/2015 5:01 PM, Dmitry Samersoff wrote:
> David,
>
> Looks good for me.
>
> -Dmitry
>
> On 2015-04-20 04:32, David Holmes wrote:
>> Hi Dmitry,
>>
>> Can we get closure on this. Are you okay with:
>>
>> http://cr.openjdk.java.net/~dholmes/8077674/webrev.v2/
>>
>> If not please advise exactly how you think this should be fixed.
>>
>> Thanks,
>> David
>>
>> On 16/04/2015 5:32 PM, David Holmes wrote:
>>> On 16/04/2015 5:00 PM, Dmitry Samersoff wrote:
>>>> David,
>>>>
>>>>> True but "normal" programming style for mutually exclusive
>>>>> conditions is to use if/else style logic.
>>>>
>>>> Agree in general, but I vote for better readability/error resistance
>>>> here.
>>>
>>> It might be more readable in the sense it is minimalist, but IMHO it is
>>> less understandable to treat mutually exclusive conditions as if they
>>> were not mutually exclusive. I don't see the error resistance argument ??
>>>
>>>> Also, you may consider to add "sanity" block to the top of file to
>>>> defend to incorrect defines (multiple variants, VAR = 0 etc)
>>>
>>> Not sure what you mean but this seems to be exploding beyond the simple
>>> build fix I was trying to quickly get out.
>>>
>>> Thanks,
>>> David
>>>
>>>> -Dmitry
>>>>
>>>>
>>>> On 2015-04-16 09:47, David Holmes wrote:
>>>>> On 16/04/2015 4:43 PM, Dmitry Samersoff wrote:
>>>>>> David,
>>>>>>
>>>>>> As __FreeBSD__ and __OpenBSD__ etc is unlikely defined together
>>>>>> you don't need #else here.
>>>>>
>>>>> True but "normal" programming style for mutually exclusive
>>>>> conditions is
>>>>> to use if/else style logic.
>>>>>
>>>>> It's a third option I suppose, but I find it less appealing.
>>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>>> i.e.
>>>>>>
>>>>>> #ifdef __FreeBSD__
>>>>>> ...
>>>>>> #endif
>>>>>>
>>>>>> #ifdef __OpenBSD__
>>>>>> ...
>>>>>> #endif
>>>>>>
>>>>>> etc.
>>>>>>
>>>>>> -Dmitry
>>>>>>
>>>>>> On 2015-04-16 08:25, David Holmes wrote:
>>>>>>> Here's an alternate webrev based on Kim's findings in the other
>>>>>>> thread.
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~dholmes/8077674/webrev.v2/
>>>>>>>
>>>>>>> Basically #elif XXX is replaced with #else #ifdef XXX #endif". With
>>>>>>> lots
>>>>>>> of new lines of course.
>>>>>>>
>>>>>>> I don't know what the normal style rules are for nesting
>>>>>>> preprocessing
>>>>>>> conditionals. I prefer to see the # lines indented, but I know people
>>>>>>> like to see the real code at the indent it would be if there were no
>>>>>>> conditional.
>>>>>>>
>>>>>>> As I wrote here:
>>>>>>>
>>>>>>> http://mail.openjdk.java.net/pipermail/build-dev/2015-April/014801.html
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> It seems to me there are two choices here:
>>>>>>>
>>>>>>> a) Use the [original] change proposed but verify that the variables
>>>>>>> concerned are ones that if defined will never be defined to be zero
>>>>>>> (which I believe to be the case but may have trouble proving :( )
>>>>>>>
>>>>>>> b) Replace the 'elif XXX' with 'else ifdef XXX' [v2]
>>>>>>>
>>>>>>> For the few cases involved here (b) seems the easier path.
>>>>>>>
>>>>>>> Comments/suggestions welcomed.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> David
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 16/04/2015 11:50 AM, David Holmes wrote:
>>>>>>>> Still need Review for this please (with plans to backport to 8u).
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> David
>>>>>>>>
>>>>>>>> On 15/04/2015 6:25 AM, David Holmes wrote:
>>>>>>>>> Hi Dmitry,
>>>>>>>>>
>>>>>>>>> On 14/04/2015 9:25 PM, Dmitry Samersoff wrote:
>>>>>>>>>> David,
>>>>>>>>>>
>>>>>>>>>> It's better to change
>>>>>>>>>>
>>>>>>>>>> #ifdef __APPLE__
>>>>>>>>>>
>>>>>>>>>> to
>>>>>>>>>>
>>>>>>>>>> #if defined(__APPLE__)
>>>>>>>>>
>>>>>>>>> Why? They seem 100% equivalent. Plus that would mean a lot of
>>>>>>>>> changes.
>>>>>>>>> Plus we use that pattern in many, many, many places.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> David
>>>>>>>>>
>>>>>>>>>> -Dmitry
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 2015-04-14 13:20, David Holmes wrote:
>>>>>>>>>>> webrev:
>>>>>>>>>>>
>>>>>>>>>>> http://cr.openjdk.java.net/~dholmes/8077674/webrev/
>>>>>>>>>>>
>>>>>>>>>>> From the original error report:
>>>>>>>>>>>> "-Wundef option causes GCC to warn whenever it encounters an
>>>>>>>>>>>> identifier
>>>>>>>>>>>> which is not a macro in an ‘#if’. "
>>>>>>>>>>>>
>>>>>>>>>>>> Hence we should be using
>>>>>>>>>>>>
>>>>>>>>>>>> #elif defined(__FreeBSD__)
>>>>>>>>>>>>
>>>>>>>>>>>> etc in this context.
>>>>>>>>>>>
>>>>>>>>>>> I also founds similar suspect usages in os_linux.cpp and
>>>>>>>>>>> os_windows.cpp.
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> David
>>>>>>>>>>
>>>>>>>>>>
>>>>>>
>>>>>>
>>>>
>>>>
>
>
More information about the hotspot-runtime-dev
mailing list